Skip to content

Commit 61a4d24

Browse files
TheLastFlamejustinmcQuncCccccc
authored
Fix visual overlap of transparent routes barrier when using FadeForwardsPageTransitionsBuilder (flutter#167032)
Reopens flutter#165726 with the removed test that has become irrelevant. Depends on: flutter#167324 Actual master: https://github.com/user-attachments/assets/28619355-9c1e-4f06-9ede-38c4dddd13df After fix: https://github.com/user-attachments/assets/a5f2ecf2-5d8e-445a-b5a9-a7d6c0e3ec5d ## 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]. - [ ] 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. --------- Co-authored-by: Justin McCandless <[email protected]> Co-authored-by: Qun Cheng <[email protected]>
1 parent dd7b213 commit 61a4d24

File tree

2 files changed

+103
-22
lines changed

2 files changed

+103
-22
lines changed

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

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import 'package:flutter/foundation.dart';
1515
import 'package:flutter/rendering.dart';
1616
import 'package:flutter/services.dart';
1717

18+
import 'color_scheme.dart';
1819
import 'colors.dart';
1920
import 'theme.dart';
2021

@@ -805,38 +806,43 @@ class FadeForwardsPageTransitionsBuilder extends PageTransitionsBuilder {
805806
Animation<double> secondaryAnimation,
806807
Color? backgroundColor,
807808
Widget? child,
808-
) => DualTransitionBuilder(
809-
animation: ReverseAnimation(secondaryAnimation),
810-
forwardBuilder: (BuildContext context, Animation<double> animation, Widget? child) {
811-
return ColoredBox(
812-
color: animation.isAnimating
813-
? backgroundColor ?? Theme.of(context).colorScheme.surface
814-
: Colors.transparent,
815-
child: FadeTransition(
809+
) {
810+
final Widget builder = DualTransitionBuilder(
811+
animation: ReverseAnimation(secondaryAnimation),
812+
forwardBuilder: (BuildContext context, Animation<double> animation, Widget? child) {
813+
return FadeTransition(
816814
opacity: _fadeInTransition.animate(animation),
817815
child: SlideTransition(
818816
position: _secondaryForwardTranslationTween.animate(animation),
819817
child: child,
820818
),
821-
),
822-
);
823-
},
824-
reverseBuilder: (BuildContext context, Animation<double> animation, Widget? child) {
825-
return ColoredBox(
826-
color: animation.isAnimating
827-
? backgroundColor ?? Theme.of(context).colorScheme.surface
828-
: Colors.transparent,
829-
child: FadeTransition(
819+
);
820+
},
821+
reverseBuilder: (BuildContext context, Animation<double> animation, Widget? child) {
822+
return FadeTransition(
830823
opacity: _fadeOutTransition.animate(animation),
831824
child: SlideTransition(
832825
position: _secondaryBackwardTranslationTween.animate(animation),
833826
child: child,
834827
),
835-
),
836-
);
837-
},
838-
child: child,
839-
);
828+
);
829+
},
830+
child: child,
831+
);
832+
833+
final bool isOpaque = ModalRoute.opaqueOf(context) ?? true;
834+
835+
if (!isOpaque) {
836+
return builder;
837+
}
838+
839+
return ColoredBox(
840+
color: secondaryAnimation.isAnimating
841+
? backgroundColor ?? ColorScheme.of(context).surface
842+
: Colors.transparent,
843+
child: builder,
844+
);
845+
}
840846

841847
@override
842848
Widget buildTransitions<T>(

packages/flutter/test/material/page_transitions_theme_test.dart

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,81 @@ void main() {
314314
},
315315
variant: TargetPlatformVariant.only(TargetPlatform.android),
316316
);
317+
318+
testWidgets(
319+
'FadeForwardsPageTransitionBuilder does not use ColoredBox for non-opaque routes',
320+
(WidgetTester tester) async {
321+
await tester.pumpWidget(
322+
MaterialApp(
323+
theme: ThemeData(
324+
pageTransitionsTheme: const PageTransitionsTheme(
325+
builders: <TargetPlatform, PageTransitionsBuilder>{
326+
TargetPlatform.android: FadeForwardsPageTransitionsBuilder(
327+
backgroundColor: Colors.lightGreen,
328+
),
329+
},
330+
),
331+
),
332+
home: Builder(
333+
builder: (BuildContext context) {
334+
return Material(
335+
child: TextButton(
336+
onPressed: () {
337+
Navigator.push(
338+
context,
339+
PageRouteBuilder<void>(
340+
opaque: false,
341+
pageBuilder: (_, _, _) {
342+
return Material(
343+
child: TextButton(
344+
onPressed: () {
345+
Navigator.push(
346+
context,
347+
MaterialPageRoute<void>(builder: (_) => const Text('page b')),
348+
);
349+
},
350+
child: const Text('push b'),
351+
),
352+
);
353+
},
354+
),
355+
);
356+
},
357+
child: const Text('push a'),
358+
),
359+
);
360+
},
361+
),
362+
),
363+
);
364+
365+
await tester.tap(find.text('push a'));
366+
await tester.pumpAndSettle();
367+
368+
await tester.tap(find.text('push b'));
369+
await tester.pump(const Duration(milliseconds: 400));
370+
371+
void findColoredBox() {
372+
expect(
373+
find.byWidgetPredicate((Widget w) => w is ColoredBox && w.color == Colors.lightGreen),
374+
findsNothing,
375+
);
376+
}
377+
378+
// Check that ColoredBox is not used for non-opaque route.
379+
findColoredBox();
380+
381+
await tester.pumpAndSettle();
382+
383+
Navigator.pop(tester.element(find.text('page b')));
384+
385+
await tester.pumpAndSettle(const Duration(milliseconds: 400));
386+
387+
// Check that ColoredBox is not used for non-opaque route
388+
findColoredBox();
389+
},
390+
variant: TargetPlatformVariant.only(TargetPlatform.android),
391+
);
317392
});
318393

319394
testWidgets(

0 commit comments

Comments
 (0)