Skip to content

[rfw] Add OverflowBar widget and Update ButtonBar widget implementation #5807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jan 30, 2024
Merged

[rfw] Add OverflowBar widget and Update ButtonBar widget implementation #5807

merged 2 commits into from
Jan 30, 2024

Conversation

TahaTesser
Copy link
Member

fixes [RFW] Replace ButtonBar with a backwards compatible implementation using OverflowBar

Description

This adds OverflowBar widget to material widgets and updates ButtonBar (to be deprecated widget) to use OverflowBar underneath for backwards compatibility.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the p: rfw Remote Flutter Widgets label Jan 5, 2024
@TahaTesser TahaTesser changed the title [rfw] Introduce OverflowBar and Replace ButtonBar with a backwards compatible implementation [rfw] Add OverflowBar widget and Update ButtonBar widget implementation Jan 5, 2024
@TahaTesser TahaTesser force-pushed the replace_button_bar branch 5 times, most recently from 68f59d2 to b65af44 Compare January 9, 2024 14:24
@TahaTesser TahaTesser marked this pull request as ready for review January 9, 2024 14:34
@TahaTesser TahaTesser requested a review from Hixie as a code owner January 9, 2024 14:34
@TahaTesser

This comment was marked as resolved.

Sign in to view
Hixie
Hixie reviewed Jan 9, 2024
View reviewed changes
Hixie
Hixie reviewed Jan 9, 2024
View reviewed changes
@Hixie
Copy link
Contributor

Hixie commented Jan 9, 2024

I don't understand the diff, can you explain it in more detail? Why is the new rendering different?

@TahaTesser TahaTesser force-pushed the replace_button_bar branch 2 times, most recently from 0958442 to cf4963f Compare January 11, 2024 09:52
@TahaTesser
Copy link
Member Author

TahaTesser commented Jan 11, 2024 •
edited
Loading

I don't understand the diff, can you explain it in more detail? Why is the new rendering different?

There is no more diff now (there was one before because I ran run_tests.sh --update-goldens on my M1 MacBook)

@TahaTesser TahaTesser requested a review from Hixie January 11, 2024 14:26
Hixie
Hixie reviewed Jan 11, 2024
View reviewed changes
@TahaTesser TahaTesser requested a review from Hixie January 16, 2024 11:48
Hixie
Hixie reviewed Jan 16, 2024
View reviewed changes
Hixie
Hixie reviewed Jan 16, 2024
View reviewed changes
@TahaTesser TahaTesser requested a review from Hixie January 18, 2024 08:20
Hixie
Hixie reviewed Jan 18, 2024
View reviewed changes
@TahaTesser TahaTesser requested a review from Hixie January 19, 2024 11:49
@TahaTesser
Copy link
Member Author

@Hixie Resolved version conflicts.

@TahaTesser TahaTesser force-pushed the replace_button_bar branch 3 times, most recently from af55e33 to f1b6bb2 Compare January 29, 2024 15:40
@TahaTesser
Copy link
Member Author

Resolved new conflicts

Hixie
Hixie reviewed Jan 29, 2024
View reviewed changes
packages/rfw/lib/src/flutter/material_widgets.dart
/// * buttonAlignedDropdown
///
/// It is recommended to use the OverflowBar widget.
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this all needs to be markdown with hyperlinks and so on (sorry if i wasn't clear in the earlier comment).

What I mean is e.g. ButtonBar needs to be [ButtonBar], package:flutter/material.dart needs to be whatever the magic incantation is to reference the material library (maybe just [material]?), rfw needs to either be in backticks or uppercase, the parameters in the list should be in backticks, etc.

Copy link
Member Author

@TahaTesser TahaTesser Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh my bad. Thanks for the suggestion!

package:flutter/material.dart can be simply referenced as "Flutter's material library". We reference like this in the framework too in various classes

Hixie
Hixie approved these changes Jan 29, 2024
View reviewed changes
Copy link
Contributor

@Hixie Hixie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo the documentation fixes requested above

@TahaTesser TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 30, 2024
@auto-submit auto-submit bot merged commit 25abb5d into flutter:main Jan 30, 2024
@TahaTesser TahaTesser deleted the replace_button_bar branch January 30, 2024 10:30
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 30, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
@engine-flutter-autoroll
25abb5d [rfw] Add OverflowBar widget and Update ButtonBar widget …
3787800 
…implementation (flutter/packages#5807)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 30, 2024
@engine-flutter-autoroll
Roll Packages from 516648a to 25abb5d (6 revisions) (#142527)
461f0bf 
flutter/packages@516648a...25abb5d

2024-01-30 [email protected] [rfw] Add `OverflowBar` widget and Update `ButtonBar` widget implementation (flutter/packages#5807)
2024-01-30 [email protected] [ci] Reduce tasks slightly (flutter/packages#6011)
2024-01-29 [email protected] Clarify issue requirement (flutter/packages#6010)
2024-01-29 [email protected] [various] Disambiguate TestDefaultBinaryMessengerBinding (flutter/packages#6009)
2024-01-29 [email protected] Manual roll Flutter from a8efa77 to 2f6fdf2 (23 revisions) (flutter/packages#6008)
2024-01-29 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.23.1 to 3.23.2 (flutter/packages#5987)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@Hixie
Copy link
Contributor

Hixie commented Jan 30, 2024

Looks like the test coverage is no longer at 100% since this PR.

@TahaTesser TahaTesser mentioned this pull request Jan 31, 2024
Closed
@TahaTesser
Copy link
Member Author

TahaTesser commented Jan 31, 2024 •
edited
Loading

Looks like the test coverage is no longer at 100% since this PR.

Filed a fix #6020

@reidbaker reidbaker mentioned this pull request Feb 19, 2024
Merged
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hixie Hixie Hixie approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: rfw Remote Flutter Widgets
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFW] Replace ButtonBar with a backwards compatible implementation using OverflowBar
2 participants
@TahaTesser @Hixie