From 300c5d828532cad079c69fada84313d09de9d12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Tue, 25 Jul 2023 11:20:31 -0700 Subject: [PATCH] Revert "Proposal to add barrier configs for showDatePicker, showTimePicker and showAboutDialog." (#131278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts flutter/flutter#130484. /cc @ronnnnn Example failure: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac%20framework_tests_libraries/12185/overview
Failure logs... ``` 04:51 +5379 ~18: /Volumes/Work/s/w/ir/x/w/flutter/packages/flutter/test/material/about_test.dart: Barrier dismissible Barrier is dismissible with default parameter ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════ The following TestFailure was thrown running a test: Expected: <1> Actual: <2> When the exception was thrown, this was the stack: #4 main.. (file:///Volumes/Work/s/w/ir/x/w/flutter/packages/flutter/test/material/about_test.dart:776:7) #5 testWidgets.. (package:flutter_test/src/widget_tester.dart:165:15) #6 TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1008:5) (elided one frame from package:stack_trace) This was caught by the test expectation on the following line: file:///Volumes/Work/s/w/ir/x/w/flutter/packages/flutter/test/material/about_test.dart line 776 The test description was: Barrier is dismissible with default parameter ════════════════════════════════════════════════════════════════════════════════════════════════════ 04:51 +5379 ~18 -1: /Volumes/Work/s/w/ir/x/w/flutter/packages/flutter/test/material/about_test.dart: Barrier dismissible Barrier is dismissible with default parameter [E] Test failed. See exception logs above. The test description was: Barrier is dismissible with default parameter To run this test again: /Volumes/Work/s/w/ir/x/w/flutter/bin/cache/dart-sdk/bin/dart test /Volumes/Work/s/w/ir/x/w/flutter/packages/flutter/test/material/about_test.dart -p vm --plain-name 'Barrier dismissible Barrier is dismissible with default parameter' ```
--- packages/flutter/lib/src/material/about.dart | 12 +- .../flutter/lib/src/material/date_picker.dart | 26 +-- packages/flutter/lib/src/material/dialog.dart | 6 +- .../flutter/lib/src/material/time_picker.dart | 11 +- .../flutter/test/material/about_test.dart | 162 --------------- .../test/material/date_picker_test.dart | 192 ------------------ .../test/material/time_picker_test.dart | 169 --------------- 7 files changed, 14 insertions(+), 564 deletions(-) diff --git a/packages/flutter/lib/src/material/about.dart b/packages/flutter/lib/src/material/about.dart index 44d4e7b1cf..280ef14b25 100644 --- a/packages/flutter/lib/src/material/about.dart +++ b/packages/flutter/lib/src/material/about.dart @@ -170,9 +170,9 @@ class AboutListTile extends StatelessWidget { /// The licenses shown on the [LicensePage] are those returned by the /// [LicenseRegistry] API, which can be used to add more licenses to the list. /// -/// The [context], [barrierDismissible], [barrierColor], [barrierLabel], -/// [useRootNavigator], [routeSettings] and [anchorPoint] arguments are -/// passed to [showDialog], the documentation for which discusses how it is used. +/// The [context], [useRootNavigator], [routeSettings] and [anchorPoint] +/// arguments are passed to [showDialog], the documentation for which discusses +/// how it is used. void showAboutDialog({ required BuildContext context, String? applicationName, @@ -180,18 +180,12 @@ void showAboutDialog({ Widget? applicationIcon, String? applicationLegalese, List? children, - bool barrierDismissible = true, - Color? barrierColor, - String? barrierLabel, bool useRootNavigator = true, RouteSettings? routeSettings, Offset? anchorPoint, }) { showDialog( context: context, - barrierDismissible: barrierDismissible, - barrierColor: barrierColor, - barrierLabel: barrierLabel, useRootNavigator: useRootNavigator, builder: (BuildContext context) { return AboutDialog( diff --git a/packages/flutter/lib/src/material/date_picker.dart b/packages/flutter/lib/src/material/date_picker.dart index 93c699cc9d..2fb95113d0 100644 --- a/packages/flutter/lib/src/material/date_picker.dart +++ b/packages/flutter/lib/src/material/date_picker.dart @@ -113,10 +113,9 @@ const double _kMaxTextScaleFactor = 1.3; /// [locale] and [textDirection] are non-null, [textDirection] overrides the /// direction chosen for the [locale]. /// -/// The [context], [barrierDismissible], [barrierColor], [barrierLabel], -/// [useRootNavigator] and [routeSettings] arguments are passed to [showDialog], -/// the documentation for which discusses how it is used. -/// [context], [barrierDismissible] and [useRootNavigator] must be non-null. +/// The [context], [useRootNavigator] and [routeSettings] arguments are passed to +/// [showDialog], the documentation for which discusses how it is used. [context] +/// and [useRootNavigator] must be non-null. /// /// The [builder] parameter can be used to wrap the dialog widget /// to add inherited widgets like [Theme]. @@ -170,9 +169,6 @@ Future showDatePicker({ String? cancelText, String? confirmText, Locale? locale, - bool barrierDismissible = true, - Color? barrierColor, - String? barrierLabel, bool useRootNavigator = true, RouteSettings? routeSettings, TextDirection? textDirection, @@ -247,9 +243,6 @@ Future showDatePicker({ return showDialog( context: context, - barrierDismissible: barrierDismissible, - barrierColor: barrierColor, - barrierLabel: barrierLabel, useRootNavigator: useRootNavigator, routeSettings: routeSettings, builder: (BuildContext context) { @@ -974,10 +967,9 @@ class _DatePickerHeader extends StatelessWidget { /// [locale] and [textDirection] are non-null, [textDirection] overrides the /// direction chosen for the [locale]. /// -/// The [context], [barrierDismissible], [barrierColor], [barrierLabel], -/// [useRootNavigator] and [routeSettings] arguments are passed to [showDialog], -/// the documentation for which discusses how it is used. -/// [context], [barrierDismissible] and [useRootNavigator] must be non-null. +/// The [context], [useRootNavigator] and [routeSettings] arguments are passed +/// to [showDialog], the documentation for which discusses how it is used. +/// [context] and [useRootNavigator] must be non-null. /// /// The [builder] parameter can be used to wrap the dialog widget /// to add inherited widgets like [Theme]. @@ -1030,9 +1022,6 @@ Future showDateRangePicker({ String? fieldStartLabelText, String? fieldEndLabelText, Locale? locale, - bool barrierDismissible = true, - Color? barrierColor, - String? barrierLabel, bool useRootNavigator = true, RouteSettings? routeSettings, TextDirection? textDirection, @@ -1111,9 +1100,6 @@ Future showDateRangePicker({ return showDialog( context: context, - barrierDismissible: barrierDismissible, - barrierColor: barrierColor, - barrierLabel: barrierLabel, useRootNavigator: useRootNavigator, routeSettings: routeSettings, useSafeArea: false, diff --git a/packages/flutter/lib/src/material/dialog.dart b/packages/flutter/lib/src/material/dialog.dart index b717e9a4a8..57300b5601 100644 --- a/packages/flutter/lib/src/material/dialog.dart +++ b/packages/flutter/lib/src/material/dialog.dart @@ -1404,7 +1404,7 @@ Future showDialog({ required BuildContext context, required WidgetBuilder builder, bool barrierDismissible = true, - Color? barrierColor, + Color? barrierColor = Colors.black54, String? barrierLabel, bool useSafeArea = true, bool useRootNavigator = true, @@ -1426,7 +1426,7 @@ Future showDialog({ return Navigator.of(context, rootNavigator: useRootNavigator).push(DialogRoute( context: context, builder: builder, - barrierColor: barrierColor ?? Colors.black54, + barrierColor: barrierColor, barrierDismissible: barrierDismissible, barrierLabel: barrierLabel, useSafeArea: useSafeArea, @@ -1449,7 +1449,7 @@ Future showAdaptiveDialog({ required BuildContext context, required WidgetBuilder builder, bool? barrierDismissible, - Color? barrierColor, + Color? barrierColor = Colors.black54, String? barrierLabel, bool useSafeArea = true, bool useRootNavigator = true, diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart index 86ebd1f3da..bde201785a 100644 --- a/packages/flutter/lib/src/material/time_picker.dart +++ b/packages/flutter/lib/src/material/time_picker.dart @@ -2874,9 +2874,8 @@ class _TimePickerState extends State<_TimePicker> with RestorationMixin { /// ``` /// {@end-tool} /// -/// The [context], [barrierDismissible], [barrierColor], [barrierLabel], -/// [useRootNavigator] and [routeSettings] arguments are passed to [showDialog], -/// the documentation for which discusses how it is used. +/// The [context], [useRootNavigator] and [routeSettings] arguments are passed +/// to [showDialog], the documentation for which discusses how it is used. /// /// The [builder] parameter can be used to wrap the dialog widget to add /// inherited widgets like [Localizations.override], [Directionality], or @@ -2955,9 +2954,6 @@ Future showTimePicker({ required BuildContext context, required TimeOfDay initialTime, TransitionBuilder? builder, - bool barrierDismissible = true, - Color? barrierColor, - String? barrierLabel, bool useRootNavigator = true, TimePickerEntryMode initialEntryMode = TimePickerEntryMode.dial, String? cancelText, @@ -2987,9 +2983,6 @@ Future showTimePicker({ ); return showDialog( context: context, - barrierDismissible: barrierDismissible, - barrierColor: barrierColor, - barrierLabel: barrierLabel, useRootNavigator: useRootNavigator, builder: (BuildContext context) { return builder == null ? dialog : builder(context, dialog); diff --git a/packages/flutter/test/material/about_test.dart b/packages/flutter/test/material/about_test.dart index ff93053939..cb7be72b99 100644 --- a/packages/flutter/test/material/about_test.dart +++ b/packages/flutter/test/material/about_test.dart @@ -742,160 +742,6 @@ void main() { expect(nestedObserver.licensePageCount, 0); }); - group('Barrier dismissible', () { - late AboutDialogObserver rootObserver; - - setUpAll(() { - rootObserver = AboutDialogObserver(); - }); - - testWidgets('Barrier is dismissible with default parameter', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - navigatorObservers: [rootObserver], - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showAboutDialog( - context: context, - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(rootObserver.dialogCount, 1); - - // Tap on the barrier. - await tester.tapAt(const Offset(10.0, 10.0)); - await tester.pumpAndSettle(); - expect(rootObserver.dialogCount, 0); - }); - - testWidgets('Barrier is not dismissible with barrierDismissible is false', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - navigatorObservers: [rootObserver], - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showAboutDialog( - context: context, - barrierDismissible: false - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(rootObserver.dialogCount, 1); - - // Tap on the barrier, which shouldn't do anything this time. - await tester.tapAt(const Offset(10.0, 10.0)); - await tester.pumpAndSettle(); - expect(rootObserver.dialogCount, 1); - }); - }); - - testWidgets('Barrier color', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showAboutDialog( - context: context, - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).color, Colors.black54); - - // Dismiss the dialog. - await tester.tapAt(const Offset(10.0, 10.0)); - - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showAboutDialog( - context: context, - barrierColor: Colors.pink, - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).color, Colors.pink); - }); - - testWidgets('Barrier Label', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showAboutDialog( - context: context, - barrierLabel: 'Custom Label', - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).semanticsLabel, 'Custom Label'); - }); - testWidgetsWithLeakTracking('showAboutDialog uses root navigator by default', (WidgetTester tester) async { final AboutDialogObserver rootObserver = AboutDialogObserver(); final AboutDialogObserver nestedObserver = AboutDialogObserver(); @@ -1895,12 +1741,4 @@ class AboutDialogObserver extends NavigatorObserver { } super.didPush(route, previousRoute); } - - @override - void didPop(Route route, Route? previousRoute) { - if (route is DialogRoute) { - dialogCount--; - } - super.didPop(route, previousRoute); - } } diff --git a/packages/flutter/test/material/date_picker_test.dart b/packages/flutter/test/material/date_picker_test.dart index 218c5f8668..3a7830806e 100644 --- a/packages/flutter/test/material/date_picker_test.dart +++ b/packages/flutter/test/material/date_picker_test.dart @@ -335,190 +335,6 @@ void main() { expect(tester.getBottomLeft(find.text('OK')).dx, 800 - ltrOkRight); }); - group('Barrier dismissible', () { - late _DatePickerObserver rootObserver; - - setUpAll(() { - rootObserver = _DatePickerObserver(); - }); - - testWidgets('Barrier is dismissible with default parameter', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - navigatorObservers: [rootObserver], - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showDatePicker( - context: context, - initialDate: DateTime.now(), - firstDate: DateTime(2018), - lastDate: DateTime(2030), - builder: (BuildContext context, - Widget? child) => const SizedBox(), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(rootObserver.datePickerCount, 1); - - // Tap on the barrier. - await tester.tapAt(const Offset(10.0, 10.0)); - await tester.pumpAndSettle(); - expect(rootObserver.datePickerCount, 0); - }); - - testWidgets('Barrier is not dismissible with barrierDismissible is false', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - navigatorObservers: [rootObserver], - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showDatePicker( - context: context, - initialDate: DateTime.now(), - firstDate: DateTime(2018), - lastDate: DateTime(2030), - barrierDismissible: false, - builder: (BuildContext context, - Widget? child) => const SizedBox(), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(rootObserver.datePickerCount, 1); - - // Tap on the barrier, which shouldn't do anything this time. - await tester.tapAt(const Offset(10.0, 10.0)); - await tester.pumpAndSettle(); - expect(rootObserver.datePickerCount, 1); - }); - }); - - testWidgets('Barrier color', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showDatePicker( - context: context, - initialDate: DateTime.now(), - firstDate: DateTime(2018), - lastDate: DateTime(2030), - builder: (BuildContext context, - Widget? child) => const SizedBox(), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).color, Colors.black54); - - // Dismiss the dialog. - await tester.tapAt(const Offset(10.0, 10.0)); - - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showDatePicker( - context: context, - barrierColor: Colors.pink, - initialDate: DateTime.now(), - firstDate: DateTime(2018), - lastDate: DateTime(2030), - builder: (BuildContext context, - Widget? child) => const SizedBox(), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).color, Colors.pink); - }); - - testWidgets('Barrier Label', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showDatePicker( - context: context, - barrierLabel: 'Custom Label', - initialDate: DateTime.now(), - firstDate: DateTime(2018), - lastDate: DateTime(2030), - builder: (BuildContext context, - Widget? child) => const SizedBox(), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).semanticsLabel, 'Custom Label'); - }); - testWidgets('uses nested navigator if useRootNavigator is false', (WidgetTester tester) async { final _DatePickerObserver rootObserver = _DatePickerObserver(); final _DatePickerObserver nestedObserver = _DatePickerObserver(); @@ -2219,12 +2035,4 @@ class _DatePickerObserver extends NavigatorObserver { } super.didPush(route, previousRoute); } - - @override - void didPop(Route route, Route? previousRoute) { - if (route is DialogRoute) { - datePickerCount--; - } - super.didPop(route, previousRoute); - } } diff --git a/packages/flutter/test/material/time_picker_test.dart b/packages/flutter/test/material/time_picker_test.dart index 5f654138f9..86a0f32d6b 100644 --- a/packages/flutter/test/material/time_picker_test.dart +++ b/packages/flutter/test/material/time_picker_test.dart @@ -668,167 +668,6 @@ void main() { expect(tester.getBottomLeft(find.text(okString)).dx, 800 - ltrOkRight); }); - group('Barrier dismissible', () { - late PickerObserver rootObserver; - - setUpAll(() { - rootObserver = PickerObserver(); - }); - - testWidgets('Barrier is dismissible with default parameter', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - navigatorObservers: [rootObserver], - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showTimePicker( - context: context, - initialTime: const TimeOfDay(hour: 7, minute: 0), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(rootObserver.pickerCount, 1); - - // Tap on the barrier. - await tester.tapAt(const Offset(10.0, 10.0)); - await tester.pumpAndSettle(); - expect(rootObserver.pickerCount, 0); - }); - - testWidgets('Barrier is not dismissible with barrierDismissible is false', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - navigatorObservers: [rootObserver], - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => - showTimePicker( - context: context, - barrierDismissible: false, - initialTime: const TimeOfDay(hour: 7, minute: 0), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(rootObserver.pickerCount, 1); - - // Tap on the barrier, which shouldn't do anything this time. - await tester.tapAt(const Offset(10.0, 10.0)); - await tester.pumpAndSettle(); - expect(rootObserver.pickerCount, 1); - }); - }); - - testWidgets('Barrier color', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showTimePicker( - context: context, - initialTime: const TimeOfDay(hour: 7, minute: 0), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).color, Colors.black54); - - // Dismiss the dialog. - await tester.tapAt(const Offset(10.0, 10.0)); - - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showTimePicker( - context: context, - barrierColor: Colors.pink, - initialTime: const TimeOfDay(hour: 7, minute: 0), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).color, Colors.pink); - }); - - testWidgets('Barrier Label', (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Builder( - builder: (BuildContext context) { - return ElevatedButton( - child: const Text('X'), - onPressed: () => showTimePicker( - context: context, - barrierLabel: 'Custom Label', - initialTime: const TimeOfDay(hour: 7, minute: 0), - ), - ); - }, - ), - ), - ), - ), - ); - - // Open the dialog. - await tester.tap(find.byType(ElevatedButton)); - await tester.pumpAndSettle(); - expect(tester.widget(find.byType(ModalBarrier).last).semanticsLabel, 'Custom Label'); - }); - testWidgets('uses root navigator by default', (WidgetTester tester) async { final PickerObserver rootObserver = PickerObserver(); final PickerObserver nestedObserver = PickerObserver(); @@ -1968,14 +1807,6 @@ class PickerObserver extends NavigatorObserver { } super.didPush(route, previousRoute); } - - @override - void didPop(Route route, Route? previousRoute) { - if (route is DialogRoute) { - pickerCount--; - } - super.didPop(route, previousRoute); - } } Future mediaQueryBoilerplate(