-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Refactor SliverAppBar.medium & SliverAppBar.large to fix several issues
#122542
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
|
Still working through this - and wow! :) |
Piinks
left a comment
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.
Is there a reason to not split these changes up? If a customer is broken by one of these changes, all of them would have to be reverted.
The reason It fixes multiple issues and doesn't split the changes, in reality, it's not a lot of changes, in fact, it removes a lot of code if you don't count the tests. Refactoring the constructor itself fixes a bunch of issues and removing collapsed text from the flexible space bar also automatically fixes a lot of redundant parameters. I could divide the PR into two, 1. refactoring constructors and 2. Removing collapsed text, if you think this will be better. |
|
Let me run it through internal testing. If some failures come up it'll probably be a good signal to split it up. |
I can see constructors are const now instead of factory and locally I had to update code sample to satisfy analyser when switching this branch and master. I suppose this will be taken care by dart fix. Is it a breaking change? |
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.
Oh nice catch! Was this test not right before without useMaterial3?
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.
Good question, please see #122542 (comment)
|
Ran internal test suite and found no breaks. 👍 |
80f7ca3 to
531ac36
Compare
|
I want to run 55fb509 through internal testing since it may change something for an existing customer. Firing off another round of tests today. 👍 |
Thank you so much! |
55fb509 to
7593b31
Compare
7593b31 to
aae73df
Compare
|
Resolved new conflicts from #122600 |
Piinks
left a comment
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.
Thank you for updating. This LGTM but please do not merge it. The Google testing works better when a PR is approved. It is not yet clear if this is completely safe to land yet and the tool is struggling to test this unapproved.
|
Hm, the update PR button is gone now. Can you rebase this? |
f09f625 to
d1fdc5b
Compare
Thank you! |
|
Ok finally, the CI issues cleared up. 😀 |
|
Woo! Good to go. Thanks for your patience. :) |
Thank you for running the tests! |
|
Wow @TahaTesser this an impressive amount of fixes in one go, congrats on getting it merged! 🥳 👍🏻 |
|
Hi. When could this fix be available on a release on the stable channel? Thanks. |
It's team work. Not easy to land such PRs without @Piinks help! |
…several issues (flutter/flutter#122542)
flutter/flutter@be45eb2...f740544 2023-04-14 [email protected] Roll Flutter Engine from b7b2e7b742aa to 4b96e38c9275 (1 revision) (flutter/flutter#124856) 2023-04-14 [email protected] Adjust repo config for VS Code formatting (flutter/flutter#122758) 2023-04-14 [email protected] Roll Flutter Engine from 4f86a05ac932 to b7b2e7b742aa (4 revisions) (flutter/flutter#124845) 2023-04-14 [email protected] Roll Flutter Engine from 00bef00f64a7 to 4f86a05ac932 (5 revisions) (flutter/flutter#124841) 2023-04-14 [email protected] Roll Flutter Engine from ad222b0c93b9 to 00bef00f64a7 (1 revision) (flutter/flutter#124837) 2023-04-14 [email protected] [flutter_tools] Reorganize android_studio_test.dart (flutter/flutter#124834) 2023-04-14 [email protected] Roll Flutter Engine from ce7be0093399 to ad222b0c93b9 (1 revision) (flutter/flutter#124832) 2023-04-14 [email protected] Remove Finder extended attributes in build target before code signing iOS frameworks (flutter/flutter#123896) 2023-04-14 [email protected] flutter-tool, web: update HTML template serviceWorkerVersion to be const (flutter/flutter#124826) 2023-04-14 [email protected] Fix `CupertinoContextMenu` throws exception on route animation (flutter/flutter#124785) 2023-04-14 [email protected] flutter-tool: enum cleanup (flutter/flutter#124760) 2023-04-14 [email protected] Roll Flutter Engine from 1289f4b513ef to ce7be0093399 (1 revision) (flutter/flutter#124825) 2023-04-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.5.1 to 3.5.2 (flutter/flutter#124822) 2023-04-13 [email protected] Roll Flutter Engine from 2ff14d733daa to 1289f4b513ef (2 revisions) (flutter/flutter#124824) 2023-04-13 [email protected] Roll Flutter Engine from eb3e7d52ebbc to 2ff14d733daa (1 revision) (flutter/flutter#124820) 2023-04-13 [email protected] i123643 print java version gradle (flutter/flutter#123644) 2023-04-13 [email protected] Roll Flutter Engine from 496543715bdb to eb3e7d52ebbc (3 revisions) (flutter/flutter#124816) 2023-04-13 [email protected] Roll pub packages (flutter/flutter#124709) 2023-04-13 [email protected] [flutter_tools] Disable flaky `output_web_test` test case (flutter/flutter#124257) 2023-04-13 [email protected] Roll Flutter Engine from 26eddb64e8ea to 496543715bdb (1 revision) (flutter/flutter#124812) 2023-04-13 [email protected] Refactor `SliverAppBar.medium` & `SliverAppBar.large` to fix several issues (flutter/flutter#122542) 2023-04-13 [email protected] Improve the docs around the TextSelectionHandleControls deprecations (flutter/flutter#123827) 2023-04-13 [email protected] Roll Packages from fe5615f to d01f4ea (5 revisions) (flutter/flutter#124795) 2023-04-13 [email protected] Roll Flutter Engine from 81a57d24e62a to 26eddb64e8ea (1 revision) (flutter/flutter#124794) 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 Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…issues (flutter#122542) Refactor `SliverAppBar.medium` & `SliverAppBar.large` to fix several issues
flutter/flutter@be45eb2...f740544 2023-04-14 [email protected] Roll Flutter Engine from b7b2e7b742aa to 4b96e38c9275 (1 revision) (flutter/flutter#124856) 2023-04-14 [email protected] Adjust repo config for VS Code formatting (flutter/flutter#122758) 2023-04-14 [email protected] Roll Flutter Engine from 4f86a05ac932 to b7b2e7b742aa (4 revisions) (flutter/flutter#124845) 2023-04-14 [email protected] Roll Flutter Engine from 00bef00f64a7 to 4f86a05ac932 (5 revisions) (flutter/flutter#124841) 2023-04-14 [email protected] Roll Flutter Engine from ad222b0c93b9 to 00bef00f64a7 (1 revision) (flutter/flutter#124837) 2023-04-14 [email protected] [flutter_tools] Reorganize android_studio_test.dart (flutter/flutter#124834) 2023-04-14 [email protected] Roll Flutter Engine from ce7be0093399 to ad222b0c93b9 (1 revision) (flutter/flutter#124832) 2023-04-14 [email protected] Remove Finder extended attributes in build target before code signing iOS frameworks (flutter/flutter#123896) 2023-04-14 [email protected] flutter-tool, web: update HTML template serviceWorkerVersion to be const (flutter/flutter#124826) 2023-04-14 [email protected] Fix `CupertinoContextMenu` throws exception on route animation (flutter/flutter#124785) 2023-04-14 [email protected] flutter-tool: enum cleanup (flutter/flutter#124760) 2023-04-14 [email protected] Roll Flutter Engine from 1289f4b513ef to ce7be0093399 (1 revision) (flutter/flutter#124825) 2023-04-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.5.1 to 3.5.2 (flutter/flutter#124822) 2023-04-13 [email protected] Roll Flutter Engine from 2ff14d733daa to 1289f4b513ef (2 revisions) (flutter/flutter#124824) 2023-04-13 [email protected] Roll Flutter Engine from eb3e7d52ebbc to 2ff14d733daa (1 revision) (flutter/flutter#124820) 2023-04-13 [email protected] i123643 print java version gradle (flutter/flutter#123644) 2023-04-13 [email protected] Roll Flutter Engine from 496543715bdb to eb3e7d52ebbc (3 revisions) (flutter/flutter#124816) 2023-04-13 [email protected] Roll pub packages (flutter/flutter#124709) 2023-04-13 [email protected] [flutter_tools] Disable flaky `output_web_test` test case (flutter/flutter#124257) 2023-04-13 [email protected] Roll Flutter Engine from 26eddb64e8ea to 496543715bdb (1 revision) (flutter/flutter#124812) 2023-04-13 [email protected] Refactor `SliverAppBar.medium` & `SliverAppBar.large` to fix several issues (flutter/flutter#122542) 2023-04-13 [email protected] Improve the docs around the TextSelectionHandleControls deprecations (flutter/flutter#123827) 2023-04-13 [email protected] Roll Packages from fe5615f to d01f4ea (5 revisions) (flutter/flutter#124795) 2023-04-13 [email protected] Roll Flutter Engine from 81a57d24e62a to 26eddb64e8ea (1 revision) (flutter/flutter#124794) 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 Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
I'm having this issue, please how can I implement your solution? |
This is currently available on the |
fixes #121511
fixes #104493
fixes #122295
fixes #115091
fixes #122530
fixes #123943
code sample
Description
This PR refactors
SliverAppBar.medium&SliverAppBar.largeand fixes a bunch of issues.buildcontext.IconButton)Fix text cut off in
SliverAppBar.mediumBefore
After
Fix title disappears with the bottom widget
Before
After
Fix title style and action theme customization (AppBarTheme) and tittle alignment
Before
After
Fix center title alignment
Before
After
Fix
automaticallyImplyLeadingBefore
After
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.