refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets#177080
Merged
auto-submit[bot] merged 2 commits intoflutter:masterfrom Oct 22, 2025
Merged
refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets#177080auto-submit[bot] merged 2 commits intoflutter:masterfrom
auto-submit[bot] merged 2 commits intoflutter:masterfrom
Conversation
github-actions
bot
added
framework
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly refactors OpenUpwardsPageTransitionsBuilder out of the material package and into the widgets package, which is a good architectural improvement. The related tests have also been moved and expanded appropriately. I've found a potential issue in the animation logic of the moved _OpenUpwardsPageTransition widget concerning the use of nested AnimatedBuilders, which could lead to stale animation values. I've provided a detailed comment with a suggested refactoring to fix this and simplify the implementation.
Comment on lines
+261
to
+295
| return AnimatedBuilder( | ||
| animation: widget.animation, | ||
| builder: (BuildContext context, Widget? child) { | ||
| return ColoredBox( | ||
| // TODO(kishan): Replace with framework color. | ||
| color: const Color(0xFF000000).withOpacity(opacityAnimation.value), | ||
| child: Align( | ||
| alignment: Alignment.bottomLeft, | ||
| child: ClipRect( | ||
| child: SizedBox( | ||
| height: clipAnimation.value, | ||
| child: OverflowBox( | ||
| alignment: Alignment.bottomLeft, | ||
| maxHeight: size.height, | ||
| child: child, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| }, | ||
| child: AnimatedBuilder( | ||
| animation: widget.secondaryAnimation, | ||
| child: FractionalTranslation( | ||
| translation: primaryTranslationAnimation.value, | ||
| child: widget.child, | ||
| ), | ||
| builder: (BuildContext context, Widget? child) { | ||
| return FractionalTranslation( | ||
| translation: secondaryTranslationAnimation.value, | ||
| child: child, | ||
| ); | ||
| }, | ||
| ), | ||
| ); |
Contributor
There was a problem hiding this comment.
The current implementation with nested AnimatedBuilders has a potential issue. The child property of an AnimatedBuilder is meant for parts of the widget tree that do not depend on the animation. However, the inner AnimatedBuilder's child is a FractionalTranslation that depends on primaryTranslationAnimation, which is derived from widget.animation. The inner AnimatedBuilder listens to widget.secondaryAnimation. When only widget.secondaryAnimation ticks, the inner builder will be called with a stale value of primaryTranslationAnimation.
While this might not cause issues if both animations are always driven together, it's incorrect usage and can lead to subtle bugs.
I suggest refactoring to use a single AnimatedBuilder that listens to both animations using Listenable.merge. This simplifies the code and ensures that all animation-dependent values are always up-to-date.
return AnimatedBuilder(
animation: Listenable.merge(<Listenable>[widget.animation, widget.secondaryAnimation]),
builder: (BuildContext context, _) {
return ColoredBox(
// TODO(kishan): Replace with framework color.
color: const Color(0xFF000000).withOpacity(opacityAnimation.value),
child: Align(
alignment: Alignment.bottomLeft,
child: ClipRect(
child: SizedBox(
height: clipAnimation.value,
child: OverflowBox(
alignment: Alignment.bottomLeft,
maxHeight: size.height,
child: FractionalTranslation(
translation: secondaryTranslationAnimation.value,
child: FractionalTranslation(
translation: primaryTranslationAnimation.value,
child: widget.child,
),
),
),
),
),
),
);
},
);
rkishan516
force-pushed
the
page-transitions-builder
branch
2 times, most recently
from
8826eb7 to
32a66fc
Compare
victorsanni
reviewed
Oct 17, 2025
Comment on lines
+261
to
+295
| return AnimatedBuilder( | ||
| animation: widget.animation, | ||
| builder: (BuildContext context, Widget? child) { | ||
| return ColoredBox( | ||
| // TODO(kishan): Replace with framework color. | ||
| color: const Color(0xFF000000).withOpacity(opacityAnimation.value), | ||
| child: Align( | ||
| alignment: Alignment.bottomLeft, | ||
| child: ClipRect( | ||
| child: SizedBox( | ||
| height: clipAnimation.value, | ||
| child: OverflowBox( | ||
| alignment: Alignment.bottomLeft, | ||
| maxHeight: size.height, | ||
| child: child, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| }, | ||
| child: AnimatedBuilder( | ||
| animation: widget.secondaryAnimation, | ||
| child: FractionalTranslation( | ||
| translation: primaryTranslationAnimation.value, | ||
| child: widget.child, | ||
| ), | ||
| builder: (BuildContext context, Widget? child) { | ||
| return FractionalTranslation( | ||
| translation: secondaryTranslationAnimation.value, | ||
| child: child, | ||
| ); | ||
| }, | ||
| ), | ||
| ); |
rkishan516
force-pushed
the
page-transitions-builder
branch
from
32a66fc to
0bc6810
Compare
victorsanni
approved these changes
Oct 20, 2025
9 tasks
MitchellGoodwin
approved these changes
Oct 22, 2025
Contributor
MitchellGoodwin
left a comment
MitchellGoodwin
left a comment
There was a problem hiding this comment.
LGTM as well! Thank you for moving this down
MitchellGoodwin
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 23, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 23, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 23, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 23, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 24, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 24, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 24, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 24, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 24, 2025
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Oct 24, 2025
Roll Flutter from 75004a6 to cb18290 (48 revisions) flutter/flutter@75004a6...cb18290 2025-10-24 [email protected] Remove unnecessary `deprecated` withOpacity in `text_button.0.dart` in examples (flutter/flutter#177374) 2025-10-24 [email protected] Roll Packages from 9ec29b6 to 53d6138 (3 revisions) (flutter/flutter#177502) 2025-10-24 [email protected] Roll Dart SDK from d3248b00f545 to a0480f399f8f (1 revision) (flutter/flutter#177498) 2025-10-24 [email protected] Roll Skia from a47931d09585 to 3ed332b77bec (3 revisions) (flutter/flutter#177487) 2025-10-24 [email protected] Roll Abseil to Chromium's 5b92b04a2e (based on Abseil commit fc4481e968) (flutter/flutter#177059) 2025-10-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Enhance PR template with changelog and impact details (#177333)" (#177468)" (flutter/flutter#177499) 2025-10-24 [email protected] Document DropdownMenu showTrailingIcon and decorationBuilder interaction (flutter/flutter#177488) 2025-10-24 [email protected] Roll Dart SDK from 4a65fb890852 to d3248b00f545 (1 revision) (flutter/flutter#177486) 2025-10-24 [email protected] Fix bottom sheet Semantics route label for mismatched platforms (flutter/flutter#177094) 2025-10-24 [email protected] Roll Skia from eba11de00d5b to a47931d09585 (5 revisions) (flutter/flutter#177481) 2025-10-24 [email protected] Fix Dialog Semantics label and flags for mismatched platforms (flutter/flutter#176781) 2025-10-24 [email protected] Fix drawer Semantics for mismatched platforms (flutter/flutter#177095) 2025-10-24 [email protected] Roll Dart SDK from f7751ccea102 to 4a65fb890852 (2 revisions) (flutter/flutter#177480) 2025-10-23 [email protected] Change the root path of the license crawl to engine/src (flutter/flutter#177352) 2025-10-23 [email protected] [Material] Change default mouse cursor of buttons to basic arrow instead of click (except on web) (flutter/flutter#171796) 2025-10-23 [email protected] [Desktop] Propagate SemanticsNode::identifier to AXPlatformNodeDelegate::AuthorUniqueId (flutter/flutter#175405) 2025-10-23 [email protected] Roll Skia from 59ef57f4104e to eba11de00d5b (5 revisions) (flutter/flutter#177461) 2025-10-23 [email protected] Roll customer tests (flutter/flutter#177409) 2025-10-23 [email protected] Update CHANGELOG 3.35.7 notes (flutter/flutter#177463) 2025-10-23 [email protected] Marks Mac module_uiscene_test_ios to be unflaky (flutter/flutter#177378) 2025-10-23 [email protected] Allow empty dart defines in `flutter assemble` (flutter/flutter#177198) 2025-10-23 [email protected] Roll Packages from d113bbc to 9ec29b6 (12 revisions) (flutter/flutter#177455) 2025-10-23 [email protected] Implements engine-side declarative pointer event handling for semantics. (flutter/flutter#176974) 2025-10-23 [email protected] Fix the platform name of the windowing_test target for macOS in ci.yaml (flutter/flutter#177472) 2025-10-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enhance PR template with changelog and impact details (#177333)" (flutter/flutter#177468) 2025-10-23 [email protected] Change Flutter APIs to use spans (flutter/flutter#177272) 2025-10-23 [email protected] Roll Dart SDK from 020988602772 to f7751ccea102 (4 revisions) (flutter/flutter#177449) 2025-10-23 [email protected] [macOS] Implement regular window (flutter/flutter#176361) 2025-10-23 [email protected] Enhance PR template with changelog and impact details (flutter/flutter#177333) 2025-10-23 [email protected] Roll Skia from 6e6acf6644ef to 59ef57f4104e (2 revisions) (flutter/flutter#177436) 2025-10-23 [email protected] Add directional static members to AlignmentGeometry. (flutter/flutter#176571) 2025-10-23 [email protected] Roll ICU from 1b2e3e8a421e to ff35c4f9df23 (5 revisions) (flutter/flutter#177434) 2025-10-23 [email protected] Adds a new CI build for Linux host DDM-enabled artifacts (flutter/flutter#177252) 2025-10-23 [email protected] Roll Skia from 920cdcadd74c to 6e6acf6644ef (1 revision) (flutter/flutter#177432) 2025-10-23 [email protected] Roll Skia from 8cd449e4953b to 920cdcadd74c (6 revisions) (flutter/flutter#177426) 2025-10-23 [email protected] Added ahem license (flutter/flutter#177423) 2025-10-23 [email protected] Roll Dart SDK from 75f6ccb9bdc5 to 020988602772 (1 revision) (flutter/flutter#177421) 2025-10-23 [email protected] [web] Set `pointer-events: none` for img-element-backed images (flutter/flutter#177418) 2025-10-22 [email protected] Remove the x64 version of build_ios_framework_module_test (flutter/flutter#177136) 2025-10-22 [email protected] Fix accessibility events not being correctly translated to ATK (flutter/flutter#176991) 2025-10-22 [email protected] Roll Skia from b55bd60ed95b to 8cd449e4953b (8 revisions) (flutter/flutter#177405) 2025-10-22 [email protected] Roll Dart SDK from c23010c4f9e6 to 75f6ccb9bdc5 (4 revisions) (flutter/flutter#177399) 2025-10-22 [email protected] Fixes crash when adding and removing mulitple page-based route (flutter/flutter#177338) 2025-10-22 [email protected] Move child parameter to end of RefreshIndicator constructor (flutter/flutter#177019) 2025-10-22 [email protected] refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (flutter/flutter#177080) 2025-10-22 [email protected] Delete stray 'text' file (flutter/flutter#177355) ...
reidbaker
pushed a commit
to AbdeMohlbi/flutter
that referenced
this pull request
Dec 10, 2025
…er#177080) Changes: * Move OpenUpwardsPageTransitionsBuilder from `material/page_transitions_theme.dart` to `widget/page_transitions_builder.dart` * Replace `Colors.black` with `const Color(0xFF000000)` in _OpenUpwardsPageTransition until framework colors are available(if we add). part of: flutter#172929 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
material/page_transitions_theme.darttowidget/page_transitions_builder.dartColors.blackwithconst Color(0xFF000000)in _OpenUpwardsPageTransition until framework colors are available(if we add).part of: #172929
Pre-launch Checklist
///).