refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets #177080

rkishan516 · 2025-10-16T03:13:36Z

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).

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.

gemini-code-assist

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.

gemini-code-assist · 2025-10-16T03:15:15Z

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

+        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,
+              );
+            },
+          ),
+        );


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, ), ), ), ), ), ), ); }, );

+1 if doable

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

victorsanni · 2025-10-17T06:46:15Z

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

+        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,
+              );
+            },
+          ),
+        );


+1 if doable

MitchellGoodwin

LGTM as well! Thank you for moving this down

flutter/flutter#177080)

Roll Flutter from 75004a6 to cb18290 (48 revisions) flutter/flutter@75004a6...cb18290 2025-10-24 116356835+AbdeMohlbi@users.noreply.github.com Remove unnecessary `deprecated` withOpacity in `text_button.0.dart‎` in examples (flutter/flutter#177374) 2025-10-24 engine-flutter-autoroll@skia.org Roll Packages from 9ec29b6 to 53d6138 (3 revisions) (flutter/flutter#177502) 2025-10-24 engine-flutter-autoroll@skia.org Roll Dart SDK from d3248b00f545 to a0480f399f8f (1 revision) (flutter/flutter#177498) 2025-10-24 engine-flutter-autoroll@skia.org Roll Skia from a47931d09585 to 3ed332b77bec (3 revisions) (flutter/flutter#177487) 2025-10-24 jason-simmons@users.noreply.github.com 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 bruno.leroux@gmail.com Document DropdownMenu showTrailingIcon and decorationBuilder interaction (flutter/flutter#177488) 2025-10-24 engine-flutter-autoroll@skia.org Roll Dart SDK from 4a65fb890852 to d3248b00f545 (1 revision) (flutter/flutter#177486) 2025-10-24 huy@nevercode.io Fix bottom sheet Semantics route label for mismatched platforms (flutter/flutter#177094) 2025-10-24 engine-flutter-autoroll@skia.org Roll Skia from eba11de00d5b to a47931d09585 (5 revisions) (flutter/flutter#177481) 2025-10-24 huy@nevercode.io Fix Dialog Semantics label and flags for mismatched platforms (flutter/flutter#176781) 2025-10-24 huy@nevercode.io Fix drawer Semantics for mismatched platforms (flutter/flutter#177095) 2025-10-24 engine-flutter-autoroll@skia.org Roll Dart SDK from f7751ccea102 to 4a65fb890852 (2 revisions) (flutter/flutter#177480) 2025-10-23 jason-simmons@users.noreply.github.com Change the root path of the license crawl to engine/src (flutter/flutter#177352) 2025-10-23 43054281+camsim99@users.noreply.github.com [Material] Change default mouse cursor of buttons to basic arrow instead of click (except on web) (flutter/flutter#171796) 2025-10-23 loic.peron@inetum.com [Desktop] Propagate SemanticsNode::identifier to AXPlatformNodeDelegate::AuthorUniqueId (flutter/flutter#175405) 2025-10-23 engine-flutter-autoroll@skia.org Roll Skia from 59ef57f4104e to eba11de00d5b (5 revisions) (flutter/flutter#177461) 2025-10-23 737941+loic-sharma@users.noreply.github.com Roll customer tests (flutter/flutter#177409) 2025-10-23 1063596+reidbaker@users.noreply.github.com Update CHANGELOG 3.35.7 notes (flutter/flutter#177463) 2025-10-23 fluttergithubbot@gmail.com Marks Mac module_uiscene_test_ios to be unflaky (flutter/flutter#177378) 2025-10-23 31859944+LongCatIsLooong@users.noreply.github.com Allow empty dart defines in `flutter assemble` (flutter/flutter#177198) 2025-10-23 engine-flutter-autoroll@skia.org Roll Packages from d113bbc to 9ec29b6 (12 revisions) (flutter/flutter#177455) 2025-10-23 zhongliu88889@gmail.com Implements engine-side declarative pointer event handling for semantics. (flutter/flutter#176974) 2025-10-23 jason-simmons@users.noreply.github.com 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 kjlubick@users.noreply.github.com Change Flutter APIs to use spans (flutter/flutter#177272) 2025-10-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 020988602772 to f7751ccea102 (4 revisions) (flutter/flutter#177449) 2025-10-23 matej.knopp@gmail.com [macOS] Implement regular window (flutter/flutter#176361) 2025-10-23 1063596+reidbaker@users.noreply.github.com Enhance PR template with changelog and impact details (flutter/flutter#177333) 2025-10-23 engine-flutter-autoroll@skia.org Roll Skia from 6e6acf6644ef to 59ef57f4104e (2 revisions) (flutter/flutter#177436) 2025-10-23 sokolovskyi.konstantin@gmail.com Add directional static members to AlignmentGeometry. (flutter/flutter#176571) 2025-10-23 engine-flutter-autoroll@skia.org Roll ICU from 1b2e3e8a421e to ff35c4f9df23 (5 revisions) (flutter/flutter#177434) 2025-10-23 goderbauer@google.com Adds a new CI build for Linux host DDM-enabled artifacts (flutter/flutter#177252) 2025-10-23 engine-flutter-autoroll@skia.org Roll Skia from 920cdcadd74c to 6e6acf6644ef (1 revision) (flutter/flutter#177432) 2025-10-23 engine-flutter-autoroll@skia.org Roll Skia from 8cd449e4953b to 920cdcadd74c (6 revisions) (flutter/flutter#177426) 2025-10-23 30870216+gaaclarke@users.noreply.github.com Added ahem license (flutter/flutter#177423) 2025-10-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 75f6ccb9bdc5 to 020988602772 (1 revision) (flutter/flutter#177421) 2025-10-23 1961493+harryterkelsen@users.noreply.github.com [web] Set `pointer-events: none` for img-element-backed images (flutter/flutter#177418) 2025-10-22 jason-simmons@users.noreply.github.com Remove the x64 version of build_ios_framework_module_test (flutter/flutter#177136) 2025-10-22 robert.ancell@canonical.com Fix accessibility events not being correctly translated to ATK (flutter/flutter#176991) 2025-10-22 engine-flutter-autoroll@skia.org Roll Skia from b55bd60ed95b to 8cd449e4953b (8 revisions) (flutter/flutter#177405) 2025-10-22 engine-flutter-autoroll@skia.org Roll Dart SDK from c23010c4f9e6 to 75f6ccb9bdc5 (4 revisions) (flutter/flutter#177399) 2025-10-22 47866232+chunhtai@users.noreply.github.com Fixes crash when adding and removing mulitple page-based route (flutter/flutter#177338) 2025-10-22 61709763+cuteUtya@users.noreply.github.com Move child parameter to end of RefreshIndicator constructor (flutter/flutter#177019) 2025-10-22 34465683+rkishan516@users.noreply.github.com refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (flutter/flutter#177080) 2025-10-22 1961493+harryterkelsen@users.noreply.github.com Delete stray 'text' file (flutter/flutter#177355) ...

…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.

rkishan516 requested a review from victorsanni October 16, 2025 03:13

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Oct 16, 2025

gemini-code-assist bot reviewed Oct 16, 2025

View reviewed changes

rkishan516 force-pushed the page-transitions-builder branch 2 times, most recently from 8826eb7 to 32a66fc Compare October 17, 2025 01:53

victorsanni reviewed Oct 17, 2025

View reviewed changes

rkishan516 added 2 commits October 17, 2025 12:28

refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets

4b94974

fix: update scrim color and animated builder nesting based on feedback

0bc6810

rkishan516 force-pushed the page-transitions-builder branch from 32a66fc to 0bc6810 Compare October 17, 2025 07:05

victorsanni requested a review from MitchellGoodwin October 20, 2025 19:40

victorsanni approved these changes Oct 20, 2025

View reviewed changes

victorsanni mentioned this pull request Oct 20, 2025

Refactor: Migrate page transition builder to widgets #174090

Draft

9 tasks

MitchellGoodwin approved these changes Oct 22, 2025

View reviewed changes

MitchellGoodwin added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 22, 2025

auto-submit bot added this pull request to the merge queue Oct 22, 2025

Merged via the queue into flutter:master with commit adb383b Oct 22, 2025
78 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 22, 2025

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

705b9d5

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 23, 2025

Roll Flutter from 75004a639ae4 to d1293aec541a (19 revisions) flutter/packages#10280

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

89c54fe

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 23, 2025

Roll Flutter from 75004a639ae4 to c52370113d8b (21 revisions) flutter/packages#10281

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

dcef94d

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 23, 2025

Roll Flutter from 75004a639ae4 to efc4d23cf0af (22 revisions) flutter/packages#10283

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

5174c80

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 23, 2025

Roll Flutter from 75004a639ae4 to efc4d23cf0af (22 revisions) flutter/packages#10285

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

1225210

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 24, 2025

Roll Flutter from 75004a639ae4 to 67068ee0fe96 (44 revisions) flutter/packages#10291

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

ef3919c

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 24, 2025

Roll Flutter from 75004a639ae4 to 9e1bf394ff3a (45 revisions) flutter/packages#10294

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

c43a6fe

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 24, 2025

Roll Flutter from 75004a639ae4 to 865f4431bd3d (46 revisions) flutter/packages#10296

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

7869921

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 24, 2025

Roll Flutter from 75004a639ae4 to b1ee775f45cb (47 revisions) flutter/packages#10297

Closed

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

adb383b refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (

b1969ff

flutter/flutter#177080)

engine-flutter-autoroll mentioned this pull request Oct 24, 2025

Roll Flutter from 75004a639ae4 to cb18290fa45e (48 revisions) flutter/packages#10299

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets #177080

refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets #177080

Uh oh!

rkishan516 commented Oct 16, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Oct 16, 2025

Uh oh!

victorsanni Oct 17, 2025

Uh oh!

Uh oh!

victorsanni Oct 17, 2025

Uh oh!

MitchellGoodwin left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets #177080

refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets #177080

Uh oh!

Conversation

rkishan516 commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

victorsanni Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

victorsanni Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rkishan516 commented Oct 16, 2025 •

edited

Loading