Skip to content

Commit 834566f

Browse files
authored
Fix ZoomPageTransitionsBuilder hardcoded fill color (flutter#154057)
Fixes: flutter#115275 Fixes: flutter#116127 Fixes: flutter#126682 Continuing on: flutter#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. <!-- Links --> [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
1 parent a3301ce commit 834566f

File tree

3 files changed

+118
-1
lines changed

3 files changed

+118
-1
lines changed

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,4 @@ Cedric Vanden Bosch <[email protected]>
128128
129129
Dimil Kalathiya <[email protected]>
130130
Nate Wilson <[email protected]>
131+

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ class _ZoomPageTransition extends StatelessWidget {
270270

271271
@override
272272
Widget build(BuildContext context) {
273+
final Color backgroundColor = Theme.of(context).colorScheme.surface;
273274
return DualTransitionBuilder(
274275
animation: animation,
275276
forwardBuilder: (
@@ -280,6 +281,7 @@ class _ZoomPageTransition extends StatelessWidget {
280281
return _ZoomEnterTransition(
281282
animation: animation,
282283
allowSnapshotting: allowSnapshotting && allowEnterRouteSnapshotting,
284+
backgroundColor: backgroundColor,
283285
child: child,
284286
);
285287
},
@@ -306,6 +308,7 @@ class _ZoomPageTransition extends StatelessWidget {
306308
animation: animation,
307309
allowSnapshotting: allowSnapshotting && allowEnterRouteSnapshotting ,
308310
reverse: true,
311+
backgroundColor: backgroundColor,
309312
child: child,
310313
);
311314
},
@@ -331,13 +334,15 @@ class _ZoomEnterTransition extends StatefulWidget {
331334
required this.animation,
332335
this.reverse = false,
333336
required this.allowSnapshotting,
337+
required this.backgroundColor,
334338
this.child,
335339
});
336340

337341
final Animation<double> animation;
338342
final Widget? child;
339343
final bool allowSnapshotting;
340344
final bool reverse;
345+
final Color backgroundColor;
341346

342347
@override
343348
State<_ZoomEnterTransition> createState() => _ZoomEnterTransitionState();
@@ -394,6 +399,7 @@ class _ZoomEnterTransitionState extends State<_ZoomEnterTransition> with _ZoomTr
394399
fade: fadeTransition,
395400
scale: scaleTransition,
396401
animation: widget.animation,
402+
backgroundColor: widget.backgroundColor,
397403
);
398404
super.initState();
399405
}
@@ -410,6 +416,7 @@ class _ZoomEnterTransitionState extends State<_ZoomEnterTransition> with _ZoomTr
410416
fade: fadeTransition,
411417
scale: scaleTransition,
412418
animation: widget.animation,
419+
backgroundColor: widget.backgroundColor,
413420
);
414421
}
415422
super.didUpdateWidget(oldWidget);
@@ -986,6 +993,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
986993
required this.scale,
987994
required this.fade,
988995
required this.animation,
996+
required this.backgroundColor,
989997
}) {
990998
animation.addListener(notifyListeners);
991999
animation.addStatusListener(_onStatusChange);
@@ -1001,6 +1009,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
10011009
final Animation<double> animation;
10021010
final Animation<double> scale;
10031011
final Animation<double> fade;
1012+
final Color backgroundColor;
10041013

10051014
final Matrix4 _transform = Matrix4.zero();
10061015
final LayerHandle<OpacityLayer> _opacityHandle = LayerHandle<OpacityLayer>();
@@ -1025,7 +1034,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
10251034
if (scrimOpacity > 0.0) {
10261035
context.canvas.drawRect(
10271036
offset & size,
1028-
Paint()..color = Colors.black.withOpacity(scrimOpacity),
1037+
Paint()..color = backgroundColor.withOpacity(scrimOpacity),
10291038
);
10301039
}
10311040
}

packages/flutter/test/material/page_transitions_theme_test.dart

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,4 +519,111 @@ void main() {
519519
expect(find.text('push'), findsOneWidget);
520520
expect(find.text('page b'), findsNothing);
521521
}, variant: TargetPlatformVariant.all());
522+
523+
testWidgets('ZoomPageTransitionsBuilder uses theme color during transition effects', (WidgetTester tester) async {
524+
// Color that is being tested for presence.
525+
const Color themeTestSurfaceColor = Color.fromARGB(255, 195, 255, 0);
526+
527+
final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
528+
'/': (BuildContext context) => Material(
529+
child: Scaffold(
530+
appBar: AppBar(
531+
title: const Text('Home Page'),
532+
),
533+
body: Center(
534+
child: Column(
535+
mainAxisAlignment: MainAxisAlignment.center,
536+
children: <Widget>[
537+
ElevatedButton(
538+
onPressed: () {
539+
Navigator.pushNamed(context, '/scaffolded');
540+
},
541+
child: const Text('Route with scaffold!'),
542+
),
543+
ElevatedButton(
544+
onPressed: () {
545+
Navigator.pushNamed(context, '/not-scaffolded');
546+
},
547+
child: const Text('Route with NO scaffold!'),
548+
),
549+
],
550+
),
551+
),
552+
),
553+
),
554+
'/scaffolded': (BuildContext context) => Material(
555+
child: Scaffold(
556+
appBar: AppBar(
557+
title: const Text('Scaffolded Page'),
558+
),
559+
body: Center(
560+
child: ElevatedButton(
561+
onPressed: () {
562+
Navigator.pop(context);
563+
},
564+
child: const Text('Back to home route...'),
565+
),
566+
),
567+
),
568+
),
569+
'/not-scaffolded': (BuildContext context) => Material(
570+
child: Center(
571+
child: ElevatedButton(
572+
onPressed: () {
573+
Navigator.pop(context);
574+
},
575+
child: const Text('Back to home route...'),
576+
),
577+
),
578+
),
579+
};
580+
581+
await tester.pumpWidget(
582+
MaterialApp(
583+
theme: ThemeData(
584+
colorScheme: ColorScheme.fromSeed(seedColor: Colors.blue, surface: themeTestSurfaceColor),
585+
pageTransitionsTheme: PageTransitionsTheme(
586+
builders: <TargetPlatform, PageTransitionsBuilder>{
587+
// Force all platforms to use ZoomPageTransitionsBuilder to test each one.
588+
for (final TargetPlatform platform in TargetPlatform.values) platform: const ZoomPageTransitionsBuilder(),
589+
},
590+
),
591+
),
592+
routes: routes,
593+
),
594+
);
595+
596+
// Go to scaffolded page.
597+
await tester.tap(find.text('Route with scaffold!'));
598+
599+
// Pump till animation is half-way through.
600+
await tester.pump();
601+
await tester.pump(const Duration(milliseconds:75));
602+
603+
// Verify that the render box is painting the right color for scaffolded pages.
604+
final RenderBox scaffoldedRenderBox = tester.firstRenderObject<RenderBox>(find.byType(MaterialApp));
605+
// Expect the color to be at exactly 12.2% opacity at this time.
606+
expect(scaffoldedRenderBox, paints..rect(color: themeTestSurfaceColor.withOpacity(0.122)));
607+
608+
await tester.pumpAndSettle();
609+
610+
// Go back home and then go to non-scaffolded page.
611+
await tester.tap(find.text('Back to home route...'));
612+
await tester.pumpAndSettle();
613+
await tester.tap(find.text('Route with NO scaffold!'));
614+
615+
// Pump till animation is half-way through.
616+
await tester.pump();
617+
await tester.pump(const Duration(milliseconds:125));
618+
619+
// Verify that the render box is painting the right color for non-scaffolded pages.
620+
final RenderBox nonScaffoldedRenderBox = tester.firstRenderObject<RenderBox>(find.byType(MaterialApp));
621+
// Expect the color to be at exactly 59.6% opacity at this time.
622+
expect(nonScaffoldedRenderBox, paints..rect(color: themeTestSurfaceColor.withOpacity(0.596)));
623+
624+
await tester.pumpAndSettle();
625+
626+
// Verify that the transition successfully completed.
627+
expect(find.text('Back to home route...'), findsOneWidget);
628+
}, variant: TargetPlatformVariant.all());
522629
}

0 commit comments

Comments
 (0)