From 9def8f6bc56b4d32975c0a5a02526bfdce7d56b7 Mon Sep 17 00:00:00 2001 From: Seiya Kokushi Date: Wed, 26 Jul 2023 00:32:20 +0900 Subject: [PATCH] Proposal to add barrier configs for showDatePicker, showTimePicker and showAboutDialog. (#130484) ### Overview Add `barrierDismissible`, `barrierColor` and `barrierLabel` parameters to `showDatePicker`, `showTimePicker` and `showAboutDialog` which calls `showDialog` internally. We can change these parameters with `showDialog` and Dialog widgets (like `DatePickerDialog`, `TimePickerDialog` or `AboutDialog`) directly. But, I think it is prefer to provide interfaces same as `showDialog` to keep application wide unified looks if it is used internally. Fixes #130971 --- 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, 564 insertions(+), 14 deletions(-) diff --git a/packages/flutter/lib/src/material/about.dart b/packages/flutter/lib/src/material/about.dart index 280ef14b25..44d4e7b1cf 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], [useRootNavigator], [routeSettings] and [anchorPoint] -/// arguments are passed to [showDialog], the documentation for which discusses -/// how it is used. +/// The [context], [barrierDismissible], [barrierColor], [barrierLabel], +/// [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,12 +180,18 @@ 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 2fb95113d0..93c699cc9d 100644 --- a/packages/flutter/lib/src/material/date_picker.dart +++ b/packages/flutter/lib/src/material/date_picker.dart @@ -113,9 +113,10 @@ const double _kMaxTextScaleFactor = 1.3; /// [locale] and [textDirection] are non-null, [textDirection] overrides the /// direction chosen for the [locale]. /// -/// 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 [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 [builder] parameter can be used to wrap the dialog widget /// to add inherited widgets like [Theme]. @@ -169,6 +170,9 @@ Future showDatePicker({ String? cancelText, String? confirmText, Locale? locale, + bool barrierDismissible = true, + Color? barrierColor, + String? barrierLabel, bool useRootNavigator = true, RouteSettings? routeSettings, TextDirection? textDirection, @@ -243,6 +247,9 @@ Future showDatePicker({ return showDialog( context: context, + barrierDismissible: barrierDismissible, + barrierColor: barrierColor, + barrierLabel: barrierLabel, useRootNavigator: useRootNavigator, routeSettings: routeSettings, builder: (BuildContext context) { @@ -967,9 +974,10 @@ class _DatePickerHeader extends StatelessWidget { /// [locale] and [textDirection] are non-null, [textDirection] overrides the /// direction chosen for the [locale]. /// -/// 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 [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 [builder] parameter can be used to wrap the dialog widget /// to add inherited widgets like [Theme]. @@ -1022,6 +1030,9 @@ Future showDateRangePicker({ String? fieldStartLabelText, String? fieldEndLabelText, Locale? locale, + bool barrierDismissible = true, + Color? barrierColor, + String? barrierLabel, bool useRootNavigator = true, RouteSettings? routeSettings, TextDirection? textDirection, @@ -1100,6 +1111,9 @@ 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 57300b5601..b717e9a4a8 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 = Colors.black54, + Color? barrierColor, 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, + barrierColor: barrierColor ?? Colors.black54, barrierDismissible: barrierDismissible, barrierLabel: barrierLabel, useSafeArea: useSafeArea, @@ -1449,7 +1449,7 @@ Future showAdaptiveDialog({ required BuildContext context, required WidgetBuilder builder, bool? barrierDismissible, - Color? barrierColor = Colors.black54, + Color? barrierColor, 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 bde201785a..86ebd1f3da 100644 --- a/packages/flutter/lib/src/material/time_picker.dart +++ b/packages/flutter/lib/src/material/time_picker.dart @@ -2874,8 +2874,9 @@ class _TimePickerState extends State<_TimePicker> with RestorationMixin { /// ``` /// {@end-tool} /// -/// The [context], [useRootNavigator] and [routeSettings] arguments are passed -/// to [showDialog], the documentation for which discusses how it is used. +/// The [context], [barrierDismissible], [barrierColor], [barrierLabel], +/// [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 @@ -2954,6 +2955,9 @@ 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, @@ -2983,6 +2987,9 @@ 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 cb7be72b99..ff93053939 100644 --- a/packages/flutter/test/material/about_test.dart +++ b/packages/flutter/test/material/about_test.dart @@ -742,6 +742,160 @@ 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(); @@ -1741,4 +1895,12 @@ 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 3a7830806e..218c5f8668 100644 --- a/packages/flutter/test/material/date_picker_test.dart +++ b/packages/flutter/test/material/date_picker_test.dart @@ -335,6 +335,190 @@ 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(); @@ -2035,4 +2219,12 @@ 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 86a0f32d6b..5f654138f9 100644 --- a/packages/flutter/test/material/time_picker_test.dart +++ b/packages/flutter/test/material/time_picker_test.dart @@ -668,6 +668,167 @@ 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(); @@ -1807,6 +1968,14 @@ 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(