Skip to content

Commit 8694baa

Browse files
committed
dialog [nfc]: showSuggestedActionDialog returns DialogStatus with result
With this API, we can use showSuggestedActionDialog in an "early return" style -- await the user's choice, and early return if they chose to dismiss the dialog. That's particularly helpful when the confirmation dialog is opened in an `if` block. We'll use this for the upcoming "edit message" feature (zulip#126), where we plan to offer a confirmation dialog before entering an edit-message session, but only if the compose box has text for a new message in it (which would be discarded if the user wants to go ahead).
1 parent 07d60a7 commit 8694baa

File tree

4 files changed

+96
-28
lines changed

4 files changed

+96
-28
lines changed

lib/widgets/app.dart

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -308,16 +308,17 @@ class ChooseAccountPage extends StatelessWidget {
308308
trailing: MenuAnchor(
309309
menuChildren: [
310310
MenuItemButton(
311-
onPressed: () {
312-
showSuggestedActionDialog(context: context,
311+
onPressed: () async {
312+
final dialog = showSuggestedActionDialog(context: context,
313313
title: zulipLocalizations.logOutConfirmationDialogTitle,
314314
message: zulipLocalizations.logOutConfirmationDialogMessage,
315315
// TODO(#1032) "destructive" style for action button
316-
actionButtonText: zulipLocalizations.logOutConfirmationDialogConfirmButton,
317-
onActionButtonPress: () {
318-
// TODO error handling if db write fails?
319-
logOutAccount(GlobalStoreWidget.of(context), accountId);
320-
});
316+
actionButtonText: zulipLocalizations.logOutConfirmationDialogConfirmButton);
317+
if (await dialog.closed == true) {
318+
if (!context.mounted) return;
319+
// TODO error handling if db write fails?
320+
unawaited(logOutAccount(GlobalStoreWidget.of(context), accountId));
321+
}
321322
},
322323
child: Text(zulipLocalizations.chooseAccountPageLogOutButton)),
323324
],

lib/widgets/compose_box.dart

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import 'dart:async';
12
import 'dart:math';
23

34
import 'package:app_settings/app_settings.dart';
@@ -907,13 +908,13 @@ Future<Iterable<_File>> _getFilePickerFiles(BuildContext context, FileType type)
907908
// If the user hasn't checked "Don't ask again", they can always dismiss
908909
// our prompt and retry, and the permissions request will reappear,
909910
// letting them grant permissions and complete the upload.
910-
showSuggestedActionDialog(context: context,
911+
final dialog = showSuggestedActionDialog(context: context,
911912
title: zulipLocalizations.permissionsNeededTitle,
912913
message: zulipLocalizations.permissionsDeniedReadExternalStorage,
913-
actionButtonText: zulipLocalizations.permissionsNeededOpenSettings,
914-
onActionButtonPress: () {
915-
AppSettings.openAppSettings();
916-
});
914+
actionButtonText: zulipLocalizations.permissionsNeededOpenSettings);
915+
if (await dialog.closed == true) {
916+
unawaited(AppSettings.openAppSettings());
917+
}
917918
} else {
918919
showErrorDialog(context: context,
919920
title: zulipLocalizations.errorDialogTitle,
@@ -1008,13 +1009,13 @@ class _AttachFromCameraButton extends _AttachUploadsButton {
10081009
// permission-request alert once, the first time the app wants to
10091010
// use a protected resource. After that, the only way the user can
10101011
// grant it is in Settings.
1011-
showSuggestedActionDialog(context: context,
1012+
final dialog = showSuggestedActionDialog(context: context,
10121013
title: zulipLocalizations.permissionsNeededTitle,
10131014
message: zulipLocalizations.permissionsDeniedCameraAccess,
1014-
actionButtonText: zulipLocalizations.permissionsNeededOpenSettings,
1015-
onActionButtonPress: () {
1016-
AppSettings.openAppSettings();
1017-
});
1015+
actionButtonText: zulipLocalizations.permissionsNeededOpenSettings);
1016+
if (await dialog.closed == true) {
1017+
unawaited(AppSettings.openAppSettings());
1018+
}
10181019
} else {
10191020
showErrorDialog(context: context,
10201021
title: zulipLocalizations.errorDialogTitle,

lib/widgets/dialog.dart

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,29 @@ Widget _dialogActionText(String text) {
1919

2020
/// Tracks the status of a dialog, in being still open or already closed.
2121
///
22+
/// Use [T] to identify the outcome of the interaction:
23+
/// - Pass `void` for an informational dialog with just the option to dismiss.
24+
/// - For confirmation dialogs with an option to dismiss
25+
/// plus an option to proceed with an action, pass `bool`.
26+
/// The action button should pass true for Navigator.pop's `result` argument.
27+
/// - For dialogs with an option to dismiss plus multiple other options,
28+
/// pass a custom enum.
29+
/// For the latter two cases, a cancel button should call Navigator.pop
30+
/// with null for the `result` argument, to match what Flutter does
31+
/// when you dismiss the dialog by tapping outside its area.
32+
///
2233
/// See also:
2334
/// * [showDialog], whose return value this class is intended to wrap.
24-
class DialogStatus {
35+
class DialogStatus<T> {
2536
const DialogStatus(this.closed);
2637

2738
/// Resolves when the dialog is closed.
28-
final Future<void> closed;
39+
///
40+
/// If this completes with null, the dialog was dismissed.
41+
/// Otherwise, completes with a [T] identifying the interaction's outcome.
42+
///
43+
/// See, e.g., [showSuggestedActionDialog] and [SuggestedActionDialogResult].
44+
final Future<T?> closed;
2945
}
3046

3147
/// Displays an [AlertDialog] with a dismiss button
@@ -37,7 +53,7 @@ class DialogStatus {
3753
// [showDialog]'s return value, a [Future], inside [DialogStatus]
3854
// whose documentation can be accessed. This helps avoid confusion when
3955
// intepreting the meaning of the [Future].
40-
DialogStatus showErrorDialog({
56+
DialogStatus<void> showErrorDialog({
4157
required BuildContext context,
4258
required String title,
4359
String? message,
@@ -61,28 +77,31 @@ DialogStatus showErrorDialog({
6177
return DialogStatus(future);
6278
}
6379

64-
void showSuggestedActionDialog({
80+
/// Displays an alert dialog with a cancel button and an action button.
81+
///
82+
/// The [DialogStatus.closed] Future gives true if the action button was tapped.
83+
/// If the dialog was canceled,
84+
/// either with the cancel button or by tapping outside the dialog's area,
85+
/// it completes with null.
86+
DialogStatus<bool> showSuggestedActionDialog({
6587
required BuildContext context,
6688
required String title,
6789
required String message,
6890
required String? actionButtonText,
69-
required VoidCallback onActionButtonPress,
7091
}) {
7192
final zulipLocalizations = ZulipLocalizations.of(context);
72-
showDialog<void>(
93+
final future = showDialog<bool>(
7394
context: context,
7495
builder: (BuildContext context) => AlertDialog(
7596
title: Text(title),
7697
content: SingleChildScrollView(child: Text(message)),
7798
actions: [
7899
TextButton(
79-
onPressed: () => Navigator.pop(context),
100+
onPressed: () => Navigator.pop<bool?>(context, null),
80101
child: _dialogActionText(zulipLocalizations.dialogCancel)),
81102
TextButton(
82-
onPressed: () {
83-
onActionButtonPress();
84-
Navigator.pop(context);
85-
},
103+
onPressed: () => Navigator.pop<bool?>(context, true),
86104
child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)),
87105
]));
106+
return DialogStatus(future);
88107
}

test/widgets/dialog_test.dart

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,51 @@ void main() {
2626
mode: LaunchMode.inAppBrowserView));
2727
});
2828
});
29+
30+
group('showSuggestedActionDialog', () {
31+
testWidgets('tap action button', (tester) async {
32+
addTearDown(testBinding.reset);
33+
await tester.pumpWidget(TestZulipApp());
34+
await tester.pump();
35+
final element = tester.element(find.byType(Placeholder));
36+
37+
final dialog = showSuggestedActionDialog(context: element,
38+
title: 'Continue?',
39+
message: 'Do the thing?',
40+
actionButtonText: 'Sure');
41+
await tester.pump();
42+
await tester.tap(find.text('Sure'));
43+
await check(dialog.closed).completes((it) => it.equals(true));
44+
});
45+
46+
testWidgets('tap cancel', (tester) async {
47+
addTearDown(testBinding.reset);
48+
await tester.pumpWidget(TestZulipApp());
49+
await tester.pump();
50+
final element = tester.element(find.byType(Placeholder));
51+
52+
final dialog = showSuggestedActionDialog(context: element,
53+
title: 'Continue?',
54+
message: 'Do the thing?',
55+
actionButtonText: 'Sure');
56+
await tester.pump();
57+
await tester.tap(find.text('Cancel'));
58+
await check(dialog.closed).completes((it) => it.equals(null));
59+
});
60+
61+
testWidgets('tap outside dialog area', (tester) async {
62+
addTearDown(testBinding.reset);
63+
await tester.pumpWidget(TestZulipApp());
64+
await tester.pump();
65+
final element = tester.element(find.byType(Placeholder));
66+
67+
final dialog = showSuggestedActionDialog(context: element,
68+
title: 'Continue?',
69+
message: 'Do the thing?',
70+
actionButtonText: 'Sure');
71+
await tester.pump();
72+
await tester.tapAt(tester.getTopLeft(find.byType(TestZulipApp)));
73+
await check(dialog.closed).completes((it) => it.equals(null));
74+
});
75+
});
2976
}

0 commit comments

Comments
 (0)