-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[flutter_adaptive_scaffold] Add breakpoint extension to get the current active breakpoint #7347
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.
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
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
@gspencergoog I just wanted to get your opinion on this before working further on it. It needs a rebase after #7300 is merged |
@gspencergoog @Renzo-Olivares do you have some more feedback on this? Otherwise i'll go ahead and add some tests and other changes required. |
@Renzo-Olivares @gspencergoog this is ready to review now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gspencergoog I know that you have already approved it, but I did add some feedback that might make @martijn00's changes a bit more concise. In other words, I added suggestions in terms of breaking up the static method into smaller components, thus making it more manageable. Please let me know if it makes sense for you to rescind your approval for these proposed suggestions. Thanks. |
@diegotori some good feedback! I've made the changes for it. @gspencergoog can you review again and let me know if you agree? |
Added one more nit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@gspencergoog looks like the PR's all green. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM again.
@Renzo-Olivares @gspencergoog What do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a few fly-by nits. Still going over the changes to pickWidget
.
@Renzo-Olivares fixed those now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I need to put this PR on hold for a bit while I check on our policies about AI-generated code. Hopefully it won't affect things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, all good to go.
(Sorry, policy has changed a lot over the last year, and I wanted to make sure we could still accept it).
@Renzo-Olivares fixed the dots. |
…he current active breakpoint (flutter/packages#7347)
…he current active breakpoint (flutter/packages#7347)
flutter/packages@86d15a6...2c37fb0 2024-08-16 [email protected] Manual roll Flutter from 99f00a1 to bced008 (13 revisions) (flutter/packages#7424) 2024-08-16 [email protected] [interactive_media_ads] Adds IMA plugin to labeler yaml (flutter/packages#7420) 2024-08-15 [email protected] Roll `avd_cipd_verison` to latest to use the `crashreport` tool. (flutter/packages#7422) 2024-08-15 [email protected] [flutter_adaptive_scaffold] Add breakpoint extension to get the current active breakpoint (flutter/packages#7347) 2024-08-15 [email protected] Roll Flutter from cc13cd1 to 99f00a1 (24 revisions) (flutter/packages#7419) 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
flutter/packages@86d15a6...2c37fb0 2024-08-16 [email protected] Manual roll Flutter from 99f00a1 to bced008 (13 revisions) (flutter/packages#7424) 2024-08-16 [email protected] [interactive_media_ads] Adds IMA plugin to labeler yaml (flutter/packages#7420) 2024-08-15 [email protected] Roll `avd_cipd_verison` to latest to use the `crashreport` tool. (flutter/packages#7422) 2024-08-15 [email protected] [flutter_adaptive_scaffold] Add breakpoint extension to get the current active breakpoint (flutter/packages#7347) 2024-08-15 [email protected] Roll Flutter from cc13cd1 to 99f00a1 (24 revisions) (flutter/packages#7419) 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
flutter/packages@86d15a6...2c37fb0 2024-08-16 [email protected] Manual roll Flutter from 99f00a1 to bced008 (13 revisions) (flutter/packages#7424) 2024-08-16 [email protected] [interactive_media_ads] Adds IMA plugin to labeler yaml (flutter/packages#7420) 2024-08-15 [email protected] Roll `avd_cipd_verison` to latest to use the `crashreport` tool. (flutter/packages#7422) 2024-08-15 [email protected] [flutter_adaptive_scaffold] Add breakpoint extension to get the current active breakpoint (flutter/packages#7347) 2024-08-15 [email protected] Roll Flutter from cc13cd1 to 99f00a1 (24 revisions) (flutter/packages#7419) 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
This enables people to check what the current breakpoint is and adjust UI based on that.
List which issues are fixed by this PR. You must list at least one issue.
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.