Fix update order of SliverAppBar #158159

hgraceb · 2024-11-05T08:26:49Z

Similar to 154484, but addressing more fundamental causes. We should call the child's didUpdateWidget before the child's build when states change.

Remove redundant code but keep the old test because we don't need it after this fix.
Add getters of lastShrinkOffset and lastOverlapsContent that are needed when SliverAppBar.update.
Add a test with a custom TabBar to avoid breaking when some changes to the default TabBar.

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 Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

nate-thegrate

@hgraceb thanks for the contribution!

The bug fix LGTM; I left some feedback on the test.

packages/flutter/test/material/app_bar_sliver_test.dart

nate-thegrate · 2024-11-05T19:37:24Z

packages/flutter/lib/src/material/tabs.dart

-        if (_currentIndex! < widget.tabs.length) {
-          final int tabIndex = _currentIndex!;
-          final Animation<double> centerAnimation = _DragAnimation(_controller!, tabIndex);
-          wrappedTabs[tabIndex] = _buildStyledTab(wrappedTabs[tabIndex], true, centerAnimation, _defaults);
-        }
+        final int tabIndex = _currentIndex!;
+        final Animation<double> centerAnimation = _DragAnimation(_controller!, tabIndex);
+        wrappedTabs[tabIndex] = _buildStyledTab(wrappedTabs[tabIndex], true, centerAnimation, _defaults);


Looks like this section is back to where it was before your change in #154485.

I'm glad both the test here and the test from that PR are passing!

packages/flutter/test/material/app_bar_sliver_test.dart

nate-thegrate

Thanks for the fix!

packages/flutter/lib/src/widgets/sliver_persistent_header.dart

Co-authored-by: Victor Sanni <victorsanniay@gmail.com>

victorsanni

LGTM pending review from someone more knowledgeable about SliverAppBar such as @Piinks.

Piinks

LGTM!

Roll Flutter from ed553d1 to 0e2d55e (40 revisions) flutter/flutter@ed553d1...0e2d55e 2024-11-15 engine-flutter-autoroll@skia.org Roll Packages from b9ac917 to b164be3 (4 revisions) (flutter/flutter#158986) 2024-11-15 magder@google.com Label PRs with gradle and Android paths "platform-android" (flutter/flutter#158970) 2024-11-15 kustermann@google.com Fix code asset copying logic in native asset code (flutter/flutter#158984) 2024-11-15 kustermann@google.com Fix duplicate work in native assets release builds (flutter/flutter#158980) 2024-11-15 matanlurey@users.noreply.github.com Stop generate both `.kts` and non-`.kts` gradle files for a test project. (flutter/flutter#158965) 2024-11-15 matanlurey@users.noreply.github.com Further skip `native_assets_test`(s) that runs `flutter build apk`. (flutter/flutter#158966) 2024-11-15 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.2.1 to 4.2.2 (flutter/flutter#157473) 2024-11-15 matanlurey@users.noreply.github.com Try running historically flaky tests first to make `flutter build apk` health tests time out more often? (flutter/flutter#158967) 2024-11-15 bkonyi@google.com [ tool ] Don't throw StateError when DDS fails to start (flutter/flutter#158744) 2024-11-14 katelovett@google.com Create merge_queue.md (flutter/flutter#158959) 2024-11-14 fluttergithubbot@gmail.com Marks Mac_x64 hot_mode_dev_cycle_macos_target__benchmark to be flaky (flutter/flutter#158569) 2024-11-14 fluttergithubbot@gmail.com Marks Mac_arm64 mac_desktop_impeller to be unflaky (flutter/flutter#158564) 2024-11-14 gengesa@gmail.com [gen_l10n] When localizing a message, prefer placeholder definitions defined by the current locale rather than the template locale (flutter/flutter#153459) 2024-11-14 august.oberhauser@swissinfo.ch feat: Include web 1.x.x in plugin template (flutter/flutter#156947) 2024-11-14 andrewrkolos@gmail.com hide members where possible (flutter/flutter#158492) 2024-11-14 andrewrkolos@gmail.com Move platform-specific log-reading implementation details from `ResidentRunner`/`FlutterDevice` to `DeviceLogReader` implementations (flutter/flutter#156181) 2024-11-14 98221114+Neutrino2711@users.noreply.github.com Updated document to clarify Clip Behaviour (flutter/flutter#157719) 2024-11-14 bkonyi@google.com Enable --verbose for android_plugin_skip_unsupported_test tests (flutter/flutter#158933) 2024-11-14 engine-flutter-autoroll@skia.org Roll Packages from 26e123a to b9ac917 (5 revisions) (flutter/flutter#158938) 2024-11-14 43054281+camsim99@users.noreply.github.com Add `dev_dependency` attribute to plugins in `.flutter-plugins-dependencies` (flutter/flutter#158009) 2024-11-14 matanlurey@users.noreply.github.com No longer pass `--verbose` to implicit `pub` calls when `flutter --verbose` is set. (flutter/flutter#158898) 2024-11-14 stuartmorgan@google.com Update triage flow chart for SVG packages (flutter/flutter#158670) 2024-11-14 bruno.leroux@gmail.com Add one MenuAnchor alignment test (flutter/flutter#158915) 2024-11-14 32538273+ValentinVignal@users.noreply.github.com Reland Add test for dynamic_content_color.0.dart (flutter/flutter#158547) 2024-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from e97b148e796d to 619804c0fbb7 (1 revision) (flutter/flutter#158905) 2024-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 82940a9aa714 to e97b148e796d (1 revision) (flutter/flutter#158901) 2024-11-14 matanlurey@users.noreply.github.com Move explicit package dependencies to a feature flag (flutter/flutter#158016) 2024-11-14 matanlurey@users.noreply.github.com Try with `bringup: true` debugging why `flutter build apk` often times out. (flutter/flutter#158895) 2024-11-14 rexios@rexios.dev Add constraint options to `SearchAnchor` suggestions builder (flutter/flutter#148856) 2024-11-14 50643541+Mairramer@users.noreply.github.com Adjusts the Hindi TimeOfDayFormat to display in a LTR orientation in localizations. (flutter/flutter#157998) 2024-11-14 38378650+hgraceb@users.noreply.github.com Fix update order of SliverAppBar (flutter/flutter#158159) 2024-11-13 42980667+srivats22@users.noreply.github.com #154792 - CupertinoActionSheetAction cursor doesn't change to clickable on desktop (flutter/flutter#158470) 2024-11-13 58190796+MitchellGoodwin@users.noreply.github.com Adds a skip message for analyzer (flutter/flutter#158890) 2024-11-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Marks Mac_arm64_ios hot_mode_dev_cycle_ios__benchmark to be flaky (#158242)" (flutter/flutter#158891) 2024-11-13 737941+loic-sharma@users.noreply.github.com [SwiftPM] Move the logic for SwiftPM enablement to the platform project (flutter/flutter#158213) 2024-11-13 matanlurey@users.noreply.github.com Temporarily skip flutter build apk for native_assets tests. (flutter/flutter#158880) 2024-11-13 me@alestiago.com docs: include Human Interface haptic information in HapticFeedback (flutter/flutter#158587) 2024-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from db3e5af2ca22 to 82940a9aa714 (2 revisions) (flutter/flutter#158799) 2024-11-13 matanlurey@users.noreply.github.com Stream the output of `flutter build` for debugging. (flutter/flutter#158757) 2024-11-13 nate.w5687@gmail.com "Fix failing checks" wiki page for new contributors (flutter/flutter#154629) 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 dit@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. ...

hgraceb added 3 commits November 4, 2024 20:27

Fix update order of SliverAppBar

8ce47b6

Remove redundant code

32c4ff7

Add test for SliverAppBar

a71c23f

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. labels Nov 5, 2024

hgraceb mentioned this pull request Nov 5, 2024

TabBar crash with SliverAppBar #158158

Closed

victorsanni requested review from Piinks, nate-thegrate and victorsanni November 5, 2024 19:22

nate-thegrate reviewed Nov 5, 2024

View reviewed changes

hgraceb and others added 2 commits November 6, 2024 21:35

Merge branch 'flutter:master' into sliver_app_bar

e28ef6e

Modify test for SliverAppBar

751d4e8

nate-thegrate approved these changes Nov 11, 2024

View reviewed changes

victorsanni reviewed Nov 12, 2024

View reviewed changes

packages/flutter/lib/src/widgets/sliver_persistent_header.dart Show resolved Hide resolved

Update packages/flutter/lib/src/widgets/sliver_persistent_header.dart

4a80b83

Co-authored-by: Victor Sanni <victorsanniay@gmail.com>

victorsanni approved these changes Nov 13, 2024

View reviewed changes

Piinks approved these changes Nov 14, 2024

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2024

auto-submit bot merged commit 5f0f18d into flutter:master Nov 14, 2024
69 checks passed

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2024

engine-flutter-autoroll mentioned this pull request Nov 14, 2024

Roll Flutter from ed553d10cf63 to 8f62a9904da0 (19 revisions) flutter/packages#8082

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

ab5a507

engine-flutter-autoroll mentioned this pull request Nov 14, 2024

Roll Flutter from ed553d10cf63 to 752ac20f8755 (20 revisions) flutter/packages#8083

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

d1f6156

engine-flutter-autoroll mentioned this pull request Nov 14, 2024

Roll Flutter from ed553d10cf63 to 46a41585ebb0 (21 revisions) flutter/packages#8085

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

efb7d0b

engine-flutter-autoroll mentioned this pull request Nov 14, 2024

Roll Flutter from ed553d10cf63 to e40306f92f0c (23 revisions) flutter/packages#8086

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

0471850

engine-flutter-autoroll mentioned this pull request Nov 15, 2024

Roll Flutter from ed553d10cf63 to 256359194e2b (37 revisions) flutter/packages#8104

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

6e4072d

engine-flutter-autoroll mentioned this pull request Nov 15, 2024

Roll Flutter from ed553d10cf63 to c4c8f6d2bd79 (39 revisions) flutter/packages#8105

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

f4a4a9d

engine-flutter-autoroll mentioned this pull request Nov 15, 2024

Roll Flutter from ed553d10cf63 to 0e2d55e0e7b2 (40 revisions) flutter/packages#8106

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2024

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

85a845c

Piinks mentioned this pull request Nov 23, 2024

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

adc99ae

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

1d62756

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

213674e

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

a086120

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

5f0f18d Fix update order of SliverAppBar (flutter/flutter#158159)

1d0721e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix update order of SliverAppBar #158159

Fix update order of SliverAppBar #158159

Uh oh!

hgraceb commented Nov 5, 2024 •

edited

Loading

Uh oh!

nate-thegrate left a comment

Uh oh!

Uh oh!

nate-thegrate Nov 5, 2024

Uh oh!

Uh oh!

nate-thegrate left a comment

Uh oh!

Uh oh!

victorsanni left a comment

Uh oh!

Piinks left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix update order of SliverAppBar #158159

Fix update order of SliverAppBar #158159

Uh oh!

Conversation

hgraceb commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nate-thegrate Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hgraceb commented Nov 5, 2024 •

edited

Loading