diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index e635071cc9..61a6462592 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -1,3 +1,5 @@ +import 'package:flutter/cupertino.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import '../generated/l10n/zulip_localizations.dart'; @@ -7,18 +9,29 @@ import 'app.dart'; import 'content.dart'; import 'store.dart'; -Widget _dialogActionText(String text) { - return Text( - text, - - // As suggested by - // https://api.flutter.dev/flutter/material/AlertDialog/actions.html : - // > It is recommended to set the Text.textAlign to TextAlign.end - // > for the Text within the TextButton, so that buttons whose - // > labels wrap to an extra line align with the overall - // > OverflowBar's alignment within the dialog. - textAlign: TextAlign.end, - ); +/// A platform-appropriate action for [AlertDialog.adaptive]'s [actions] param. +Widget _adaptiveAction({required VoidCallback onPressed, required String text}) { + switch (defaultTargetPlatform) { + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + return TextButton( + onPressed: onPressed, + child: Text( + text, + // As suggested by + // https://api.flutter.dev/flutter/material/AlertDialog/actions.html : + // > It is recommended to set the Text.textAlign to TextAlign.end + // > for the Text within the TextButton, so that buttons whose + // > labels wrap to an extra line align with the overall + // > OverflowBar's alignment within the dialog. + textAlign: TextAlign.end)); + + case TargetPlatform.iOS: + case TargetPlatform.macOS: + return CupertinoDialogAction(onPressed: onPressed, child: Text(text)); + } } /// Tracks the status of a dialog, in being still open or already closed. @@ -71,17 +84,17 @@ DialogStatus showErrorDialog({ final zulipLocalizations = ZulipLocalizations.of(context); final future = showDialog( context: context, - builder: (BuildContext context) => AlertDialog( + builder: (BuildContext context) => AlertDialog.adaptive( title: Text(title), content: message != null ? SingleChildScrollView(child: Text(message)) : null, actions: [ if (learnMoreButtonUrl != null) - TextButton( + _adaptiveAction( onPressed: () => PlatformActions.launchUrl(context, learnMoreButtonUrl), - child: _dialogActionText(zulipLocalizations.errorDialogLearnMore)), - TextButton( + text: zulipLocalizations.errorDialogLearnMore), + _adaptiveAction( onPressed: () => Navigator.pop(context), - child: _dialogActionText(zulipLocalizations.errorDialogContinue)), + text: zulipLocalizations.errorDialogContinue), ])); return DialogStatus(future); } @@ -103,16 +116,16 @@ DialogStatus showSuggestedActionDialog({ final zulipLocalizations = ZulipLocalizations.of(context); final future = showDialog( context: context, - builder: (BuildContext context) => AlertDialog( + builder: (BuildContext context) => AlertDialog.adaptive( title: Text(title), content: SingleChildScrollView(child: Text(message)), actions: [ - TextButton( + _adaptiveAction( onPressed: () => Navigator.pop(context, null), - child: _dialogActionText(zulipLocalizations.dialogCancel)), - TextButton( + text: zulipLocalizations.dialogCancel), + _adaptiveAction( onPressed: () => Navigator.pop(context, true), - child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)), + text: actionButtonText ?? zulipLocalizations.dialogContinue), ])); return DialogStatus(future); } @@ -164,7 +177,7 @@ class UpgradeWelcomeDialog extends StatelessWidget { @override Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); - return AlertDialog( + return AlertDialog.adaptive( title: Text(zulipLocalizations.upgradeWelcomeDialogTitle), content: SingleChildScrollView( child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ @@ -177,8 +190,9 @@ class UpgradeWelcomeDialog extends StatelessWidget { zulipLocalizations.upgradeWelcomeDialogLinkText)), ])), actions: [ - TextButton(onPressed: () => Navigator.pop(context), - child: Text(zulipLocalizations.upgradeWelcomeDialogDismiss)), + _adaptiveAction( + onPressed: () => Navigator.pop(context), + text: zulipLocalizations.upgradeWelcomeDialogDismiss) ]); } } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index d65ac20507..f673ebc0d8 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -335,7 +335,7 @@ void main() { await tester.tap(findButtonForLabel('Mark channel as read')); await tester.pumpAndSettle(); checkRequest(someChannel.streamId); - checkNoErrorDialog(tester); + checkNoDialog(tester); }); testWidgets('request fails', (tester) async { @@ -776,7 +776,7 @@ void main() { await tester.tap(findButtonForLabel('Mark as resolved')); await tester.pumpAndSettle(); - checkNoErrorDialog(tester); + checkNoDialog(tester); checkRequest(message.id, '✔ zulip'); }); @@ -791,7 +791,7 @@ void main() { await tester.tap(findButtonForLabel('Mark as resolved')); await tester.pumpAndSettle(); - checkNoErrorDialog(tester); + checkNoDialog(tester); checkRequest(message.id, '✔ zulip'); }); @@ -805,7 +805,7 @@ void main() { await tester.tap(findButtonForLabel('Mark as unresolved')); await tester.pumpAndSettle(); - checkNoErrorDialog(tester); + checkNoDialog(tester); checkRequest(message.id, 'zulip'); }); @@ -819,7 +819,7 @@ void main() { await tester.tap(findButtonForLabel('Mark as unresolved')); await tester.pumpAndSettle(); - checkNoErrorDialog(tester); + checkNoDialog(tester); checkRequest(message.id, 'zulip'); }); diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index 53c959321e..eca2f95cab 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -401,14 +401,14 @@ void main() { check(ZulipApp.ready).value.isFalse(); await tester.pump(); check(findSnackBarByText(message).evaluate()).isEmpty(); - checkNoErrorDialog(tester); + checkNoDialog(tester); check(ZulipApp.ready).value.isTrue(); // After app startup, reportErrorToUserBriefly displays a SnackBar. reportErrorToUserBriefly(message, details: details); await tester.pumpAndSettle(); check(findSnackBarByText(message).evaluate()).single; - checkNoErrorDialog(tester); + checkNoDialog(tester); // Open the error details dialog. await tester.tap(find.text('Details')); @@ -493,7 +493,7 @@ void main() { reportErrorToUserModally(title, message: message); check(ZulipApp.ready).value.isFalse(); await tester.pump(); - checkNoErrorDialog(tester); + checkNoDialog(tester); check(ZulipApp.ready).value.isTrue(); // After app startup, reportErrorToUserModally displays an [AlertDialog]. diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index d79acce925..e6b3cf6760 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -395,7 +395,7 @@ void main() { await prepareWithContent(tester, makeStringWithCodePoints(kMaxMessageLengthCodePoints)); await tapSendButton(tester); - checkNoErrorDialog(tester); + checkNoDialog(tester); }); testWidgets('code points not counted unnecessarily', (tester) async { @@ -434,7 +434,7 @@ void main() { await prepareWithTopic(tester, makeStringWithCodePoints(kMaxTopicLengthCodePoints)); await tapSendButton(tester); - checkNoErrorDialog(tester); + checkNoDialog(tester); }); testWidgets('code points not counted unnecessarily', (tester) async { @@ -938,7 +938,7 @@ void main() { await setupAndTapSend(tester, prepareResponse: (int messageId) { connection.prepare(json: SendMessageResult(id: messageId).toJson()); }); - checkNoErrorDialog(tester); + checkNoDialog(tester); }); testWidgets('ZulipApiException', (tester) async { @@ -1078,7 +1078,7 @@ void main() { check(call.allowMultiple).equals(true); check(call.type).equals(FileType.media); - checkNoErrorDialog(tester); + checkNoDialog(tester); check(controller!.content.text) .equals('see image: [Uploading image.jpg…]()\n\n'); @@ -1137,7 +1137,7 @@ void main() { check(call.source).equals(ImageSource.camera); check(call.requestFullMetadata).equals(false); - checkNoErrorDialog(tester); + checkNoDialog(tester); check(controller!.content.text) .equals('see image: [Uploading image.jpg…]()\n\n'); @@ -1864,7 +1864,7 @@ void main() { UploadFileResult(url: '/path/file.jpg').toJson()); await tester.tap(find.byIcon(ZulipIcons.attach_file), warnIfMissed: false); await tester.pump(Duration.zero); - checkNoErrorDialog(tester); + checkNoDialog(tester); check(testBinding.takePickFilesCalls()).length.equals(1); connection.takeRequests(); // upload request diff --git a/test/widgets/dialog_checks.dart b/test/widgets/dialog_checks.dart index 86acec96ce..20ff0effd2 100644 --- a/test/widgets/dialog_checks.dart +++ b/test/widgets/dialog_checks.dart @@ -1,10 +1,12 @@ import 'package:checks/checks.dart'; +import 'package:flutter/cupertino.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/widgets/dialog.dart'; -/// In a widget test, check that showErrorDialog was called with the right text. +/// In a widget test, check that [showErrorDialog] was called with the right text. /// /// Checks for an error dialog matching an expected title /// and, optionally, matching an expected message. Fails if none is found. @@ -15,24 +17,41 @@ Widget checkErrorDialog(WidgetTester tester, { required String expectedTitle, String? expectedMessage, }) { - final dialog = tester.widget(find.byType(AlertDialog)); - tester.widget(find.descendant(matchRoot: true, - of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); - if (expectedMessage != null) { - tester.widget(find.descendant(matchRoot: true, - of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); - } - - // TODO check "Learn more" button? + switch (defaultTargetPlatform) { + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + final dialog = tester.widget(find.bySubtype()); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); + if (expectedMessage != null) { + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); + } + return tester.widget(find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(TextButton, 'OK'))); - return tester.widget( - find.descendant(of: find.byWidget(dialog), - matching: find.widgetWithText(TextButton, 'OK'))); + case TargetPlatform.iOS: + case TargetPlatform.macOS: + final dialog = tester.widget(find.byType(CupertinoAlertDialog)); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); + if (expectedMessage != null) { + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); + } + return tester.widget(find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(CupertinoDialogAction, 'OK'))); + } } -// TODO(#996) update this to check for per-platform flavors of alert dialog -void checkNoErrorDialog(WidgetTester tester) { - check(find.byType(AlertDialog)).findsNothing(); +/// Checks that there is no dialog. +/// Fails if one is found. +void checkNoDialog(WidgetTester tester) { + check(find.byType(Dialog)).findsNothing(); + check(find.bySubtype()).findsNothing(); + check(find.byType(CupertinoAlertDialog)).findsNothing(); } /// In a widget test, check that [showSuggestedActionDialog] was called @@ -49,19 +68,35 @@ void checkNoErrorDialog(WidgetTester tester) { required String expectedMessage, String? expectedActionButtonText, }) { - final dialog = tester.widget(find.byType(AlertDialog)); - tester.widget(find.descendant(matchRoot: true, - of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); - tester.widget(find.descendant(matchRoot: true, - of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); + switch (defaultTargetPlatform) { + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + final dialog = tester.widget(find.bySubtype()); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); - final actionButton = tester.widget( - find.descendant(of: find.byWidget(dialog), - matching: find.widgetWithText(TextButton, expectedActionButtonText ?? 'Continue'))); + final actionButton = tester.widget(find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(TextButton, expectedActionButtonText ?? 'Continue'))); + final cancelButton = tester.widget(find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(TextButton, 'Cancel'))); + return (actionButton, cancelButton); - final cancelButton = tester.widget( - find.descendant(of: find.byWidget(dialog), - matching: find.widgetWithText(TextButton, 'Cancel'))); + case TargetPlatform.iOS: + case TargetPlatform.macOS: + final dialog = tester.widget(find.byType(CupertinoAlertDialog)); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); - return (actionButton, cancelButton); + final actionButton = tester.widget(find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(CupertinoDialogAction, expectedActionButtonText ?? 'Continue'))); + final cancelButton = tester.widget(find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(CupertinoDialogAction, 'Cancel'))); + return (actionButton, cancelButton); + } } diff --git a/test/widgets/dialog_test.dart b/test/widgets/dialog_test.dart index 1980f619f3..855affbb3f 100644 --- a/test/widgets/dialog_test.dart +++ b/test/widgets/dialog_test.dart @@ -1,30 +1,69 @@ import 'package:checks/checks.dart'; -import 'package:flutter/widgets.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:url_launcher/url_launcher.dart'; import 'package:zulip/widgets/dialog.dart'; import '../model/binding.dart'; +import 'dialog_checks.dart'; import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); + late BuildContext context; + + const title = "Dialog Title"; + const message = "Dialog message."; + + Future prepare(WidgetTester tester) async { + addTearDown(testBinding.reset); + + await tester.pumpWidget(const TestZulipApp( + child: Scaffold(body: Placeholder()))); + await tester.pump(); + context = tester.element(find.byType(Placeholder)); + } + group('showErrorDialog', () { - testWidgets('tap "Learn more" button', (tester) async { - addTearDown(testBinding.reset); - await tester.pumpWidget(TestZulipApp()); + testWidgets('show error dialog', (tester) async { + await prepare(tester); + + showErrorDialog(context: context, title: title, message: message); await tester.pump(); - final element = tester.element(find.byType(Placeholder)); + checkErrorDialog(tester, expectedTitle: title, expectedMessage: message); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); - showErrorDialog(context: element, title: 'hello', - learnMoreButtonUrl: Uri.parse('https://foo.example')); + testWidgets('user closes error dialog', (tester) async { + await prepare(tester); + + showErrorDialog(context: context, title: title, message: message); + await tester.pump(); + + final button = checkErrorDialog(tester, expectedTitle: title); + await tester.tap(find.byWidget(button)); + await tester.pump(); + checkNoDialog(tester); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); + + testWidgets('tap "Learn more" button', (tester) async { + await prepare(tester); + + final learnMoreButtonUrl = Uri.parse('https://foo.example'); + showErrorDialog(context: context, title: title, learnMoreButtonUrl: learnMoreButtonUrl); await tester.pump(); + checkErrorDialog(tester, expectedTitle: title); + await tester.tap(find.text('Learn more')); - check(testBinding.takeLaunchUrlCalls()).single.equals(( - url: Uri.parse('https://foo.example'), - mode: LaunchMode.inAppBrowserView)); - }); + final expectedMode = switch (defaultTargetPlatform) { + TargetPlatform.android => LaunchMode.inAppBrowserView, + TargetPlatform.iOS => LaunchMode.externalApplication, + _ => throw StateError('attempted to test with $defaultTargetPlatform'), + }; + check(testBinding.takeLaunchUrlCalls()).single + .equals((url: learnMoreButtonUrl, mode: expectedMode)); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); }); group('showSuggestedActionDialog', () { @@ -41,7 +80,7 @@ void main() { await tester.pump(); await tester.tap(find.text('Sure')); await check(dialog.result).completes((it) => it.equals(true)); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('tap cancel', (tester) async { addTearDown(testBinding.reset); @@ -56,7 +95,7 @@ void main() { await tester.pump(); await tester.tap(find.text('Cancel')); await check(dialog.result).completes((it) => it.equals(null)); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('tap outside dialog area', (tester) async { addTearDown(testBinding.reset); @@ -71,7 +110,7 @@ void main() { await tester.pump(); await tester.tapAt(tester.getTopLeft(find.byType(TestZulipApp))); await check(dialog.result).completes((it) => it.equals(null)); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); }); // TODO(#1594): test UpgradeWelcomeDialog