Fix ZoomPageTransitionsBuilder hardcoded fill color (#154057)

Fixes: #115275 Fixes: #116127 Fixes: #126682 Continuing on: #139078 (Credits to @LowLevelSubmarine for his initial work!) When using `ZoomPageTransitionsBuilder`, which is the default for `ThemeData` with a `MaterialApp`, dark edges would show around the exiting page that was being zoomed out in the background. Other times, a scrim (what looked like a slightly transparent dark overlay over the page) would appear. After some experimenting it was concluded that, in the first case, this was because both pages don't fully fill the enclosing scaffold area during the transition and the color for filling the remaining space was set hard coded as `Colors.black`. The second case (scrim) happens when navigating from a page with an enclosing scaffold to a nested one, without a scaffold, unlike the first case that happens when both pages have a (different) enclosing scaffold, except this time it would be the hard coded color covering the page with a slight opacity reduction. ### Changes - Replaced the hard coded color for transition filling with the current `ThemeData.colorScheme.surface` - Added a RenderBox based test to verify the correct color is being used in the transition. ## Preview **Before, notice the dark outline flash when navigating to the first page and the scrim when navigating to the second:** https://github.com/user-attachments/assets/b4cc8658-1008-49f4-8553-abd5fcc72989 **After, using the theme relative color (in this case the default white) to replace the hard coded value:** https://github.com/user-attachments/assets/b70f42d2-6246-4964-99d1-34ff8051ab06 ## 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. - [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.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
2024-09-05 18:57:17 +01:00 · 2024-09-05 18:57:17 +01:00 · 834566f05d
commit 834566f05d
parent a3301cecae
3 changed files with 118 additions and 1 deletions
--- a/1
+++ b/1
@ -128,3 +128,4 @@ Cedric Vanden Bosch <cedvdb@youvision.dev>
 Flop <kukuzuo@gmail.com>
 Dimil Kalathiya <kalathiyadimil@gmail.com>
 Nate Wilson <nate.w5687@gmail.com>
+dy0gu <support@dy0gu.com>
--- a/packages/flutter/lib/src/material/page_transitions_theme.dart
+++ b/packages/flutter/lib/src/material/page_transitions_theme.dart
@ -270,6 +270,7 @@ class _ZoomPageTransition extends StatelessWidget {

  @override
  Widget build(BuildContext context) {
+    final Color backgroundColor = Theme.of(context).colorScheme.surface;
    return DualTransitionBuilder(
      animation: animation,
      forwardBuilder: (
@ -280,6 +281,7 @@ class _ZoomPageTransition extends StatelessWidget {
        return _ZoomEnterTransition(
          animation: animation,
          allowSnapshotting: allowSnapshotting && allowEnterRouteSnapshotting,
+          backgroundColor: backgroundColor,
          child: child,
        );
      },
@ -306,6 +308,7 @@ class _ZoomPageTransition extends StatelessWidget {
            animation: animation,
            allowSnapshotting: allowSnapshotting && allowEnterRouteSnapshotting ,
            reverse: true,
+            backgroundColor: backgroundColor,
            child: child,
          );
        },
@ -331,6 +334,7 @@ class _ZoomEnterTransition extends StatefulWidget {
    required this.animation,
    this.reverse = false,
    required this.allowSnapshotting,
+    required this.backgroundColor,
    this.child,
  });

@ -338,6 +342,7 @@ class _ZoomEnterTransition extends StatefulWidget {
  final Widget? child;
  final bool allowSnapshotting;
  final bool reverse;
+  final Color backgroundColor;

  @override
  State<_ZoomEnterTransition> createState() => _ZoomEnterTransitionState();
@ -394,6 +399,7 @@ class _ZoomEnterTransitionState extends State<_ZoomEnterTransition> with _ZoomTr
      fade: fadeTransition,
      scale: scaleTransition,
      animation: widget.animation,
+      backgroundColor: widget.backgroundColor,
    );
    super.initState();
  }
@ -410,6 +416,7 @@ class _ZoomEnterTransitionState extends State<_ZoomEnterTransition> with _ZoomTr
        fade: fadeTransition,
        scale: scaleTransition,
        animation: widget.animation,
+        backgroundColor: widget.backgroundColor,
      );
    }
    super.didUpdateWidget(oldWidget);
@ -986,6 +993,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
    required this.scale,
    required this.fade,
    required this.animation,
+    required this.backgroundColor,
  }) {
    animation.addListener(notifyListeners);
    animation.addStatusListener(_onStatusChange);
@ -1001,6 +1009,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
  final Animation<double> animation;
  final Animation<double> scale;
  final Animation<double> fade;
+  final Color backgroundColor;

  final Matrix4 _transform = Matrix4.zero();
  final LayerHandle<OpacityLayer> _opacityHandle = LayerHandle<OpacityLayer>();
@ -1025,7 +1034,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
    if (scrimOpacity > 0.0) {
      context.canvas.drawRect(
        offset & size,
-        Paint()..color = Colors.black.withOpacity(scrimOpacity),
+        Paint()..color = backgroundColor.withOpacity(scrimOpacity),
      );
    }
  }
--- a/packages/flutter/test/material/page_transitions_theme_test.dart
+++ b/packages/flutter/test/material/page_transitions_theme_test.dart
@ -519,4 +519,111 @@ void main() {
    expect(find.text('push'), findsOneWidget);
    expect(find.text('page b'), findsNothing);
  }, variant: TargetPlatformVariant.all());
+
+  testWidgets('ZoomPageTransitionsBuilder uses theme color during transition effects', (WidgetTester tester) async {
+    // Color that is being tested for presence.
+    const Color themeTestSurfaceColor = Color.fromARGB(255, 195, 255, 0);
+
+    final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
+      '/': (BuildContext context) => Material(
+        child: Scaffold(
+          appBar: AppBar(
+            title: const Text('Home Page'),
+          ),
+          body: Center(
+            child: Column(
+              mainAxisAlignment: MainAxisAlignment.center,
+              children: <Widget>[
+                ElevatedButton(
+                  onPressed: () {
+                    Navigator.pushNamed(context, '/scaffolded');
+                  },
+                  child: const Text('Route with scaffold!'),
+                ),
+                ElevatedButton(
+                  onPressed: () {
+                    Navigator.pushNamed(context, '/not-scaffolded');
+                  },
+                  child: const Text('Route with NO scaffold!'),
+                ),
+              ],
+            ),
+          ),
+        ),
+      ),
+      '/scaffolded': (BuildContext context) => Material(
+        child: Scaffold(
+          appBar: AppBar(
+            title: const Text('Scaffolded Page'),
+          ),
+          body: Center(
+            child: ElevatedButton(
+              onPressed: () {
+                Navigator.pop(context);
+              },
+              child: const Text('Back to home route...'),
+            ),
+          ),
+        ),
+      ),
+      '/not-scaffolded': (BuildContext context) => Material(
+        child: Center(
+          child: ElevatedButton(
+            onPressed: () {
+              Navigator.pop(context);
+            },
+            child: const Text('Back to home route...'),
+          ),
+        ),
+      ),
+    };
+
+    await tester.pumpWidget(
+      MaterialApp(
+        theme: ThemeData(
+          colorScheme: ColorScheme.fromSeed(seedColor: Colors.blue, surface: themeTestSurfaceColor),
+          pageTransitionsTheme: PageTransitionsTheme(
+            builders: <TargetPlatform, PageTransitionsBuilder>{
+              // Force all platforms to use ZoomPageTransitionsBuilder to test each one.
+              for (final TargetPlatform platform in TargetPlatform.values) platform: const ZoomPageTransitionsBuilder(),
+            },
+          ),
+        ),
+        routes: routes,
+      ),
+    );
+
+    // Go to scaffolded page.
+    await tester.tap(find.text('Route with scaffold!'));
+
+    // Pump till animation is half-way through.
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds:75));
+
+    // Verify that the render box is painting the right color for scaffolded pages.
+    final RenderBox scaffoldedRenderBox = tester.firstRenderObject<RenderBox>(find.byType(MaterialApp));
+    // Expect the color to be at exactly 12.2% opacity at this time.
+    expect(scaffoldedRenderBox, paints..rect(color: themeTestSurfaceColor.withOpacity(0.122)));
+
+    await tester.pumpAndSettle();
+
+    // Go back home and then go to non-scaffolded page.
+    await tester.tap(find.text('Back to home route...'));
+    await tester.pumpAndSettle();
+    await tester.tap(find.text('Route with NO scaffold!'));
+
+    // Pump till animation is half-way through.
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds:125));
+
+    // Verify that the render box is painting the right color for non-scaffolded pages.
+    final RenderBox nonScaffoldedRenderBox = tester.firstRenderObject<RenderBox>(find.byType(MaterialApp));
+    // Expect the color to be at exactly 59.6% opacity at this time.
+    expect(nonScaffoldedRenderBox, paints..rect(color: themeTestSurfaceColor.withOpacity(0.596)));
+
+    await tester.pumpAndSettle();
+
+    // Verify that the transition successfully completed.
+    expect(find.text('Back to home route...'), findsOneWidget);
+  }, variant: TargetPlatformVariant.all());
 }