Skip to content

Commit c74d3cc

Browse files
committed
dialog [nfc]: Wrap showErrorDialog's return value
As a result, showErrorDialog no longer returns a Future that can be mistakenly awaited. The wrapped return value offers clarity on how the Future is supposed to be used. There is no user visible change because existing callers (other than lightbox's) that wait for its return value are all asynchronous handlers for UI elements like buttons, and waits unnecessarily. See also discussion #937 (comment). Signed-off-by: Zixuan James Li <[email protected]>
1 parent 6979b50 commit c74d3cc

File tree

6 files changed

+35
-16
lines changed

6 files changed

+35
-16
lines changed

lib/widgets/action_sheet.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton {
120120
default:
121121
}
122122

123-
await showErrorDialog(context: context,
123+
showErrorDialog(context: context,
124124
title: 'Adding reaction failed', message: errorMessage);
125125
}
126126
}
@@ -165,7 +165,7 @@ class StarButton extends MessageActionSheetMenuItemButton {
165165
default:
166166
}
167167

168-
await showErrorDialog(context: messageListContext,
168+
showErrorDialog(context: messageListContext,
169169
title: switch(op) {
170170
UpdateMessageFlagsOp.remove => zulipLocalizations.errorUnstarMessageFailedTitle,
171171
UpdateMessageFlagsOp.add => zulipLocalizations.errorStarMessageFailedTitle,
@@ -215,7 +215,7 @@ Future<String?> fetchRawContentWithFeedback({
215215
// TODO(?) give no feedback on error conditions we expect to
216216
// flag centrally in event polling, like invalid auth,
217217
// user/realm deactivated. (Support with reusable code.)
218-
await showErrorDialog(context: context,
218+
showErrorDialog(context: context,
219219
title: errorDialogTitle, message: errorMessage);
220220
}
221221

@@ -423,7 +423,7 @@ class ShareButton extends MessageActionSheetMenuItemButton {
423423
// Until we learn otherwise, assume something wrong happened.
424424
case ShareResultStatus.unavailable:
425425
if (!messageListContext.mounted) return;
426-
await showErrorDialog(context: messageListContext,
426+
showErrorDialog(context: messageListContext,
427427
title: zulipLocalizations.errorSharingFailed);
428428
case ShareResultStatus.success:
429429
case ShareResultStatus.dismissed:

lib/widgets/actions.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
2727
return;
2828
} catch (e) {
2929
if (!context.mounted) return;
30-
await showErrorDialog(context: context,
30+
showErrorDialog(context: context,
3131
title: zulipLocalizations.errorMarkAsReadFailedTitle,
3232
message: e.toString()); // TODO(#741): extract user-facing message better
3333
return;

lib/widgets/content.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,7 @@ class GlobalTime extends StatelessWidget {
11621162
}
11631163

11641164
void _launchUrl(BuildContext context, String urlString) async {
1165-
Future<void> showError(BuildContext context, String? message) {
1165+
DialogStatusController showError(BuildContext context, String? message) {
11661166
return showErrorDialog(context: context,
11671167
title: 'Unable to open link',
11681168
message: [
@@ -1174,7 +1174,7 @@ void _launchUrl(BuildContext context, String urlString) async {
11741174
final store = PerAccountStoreWidget.of(context);
11751175
final url = store.tryResolveUrl(urlString);
11761176
if (url == null) { // TODO(log)
1177-
await showError(context, null);
1177+
showError(context, null);
11781178
return;
11791179
}
11801180

@@ -1203,7 +1203,7 @@ void _launchUrl(BuildContext context, String urlString) async {
12031203
}
12041204
if (!launched) { // TODO(log)
12051205
if (!context.mounted) return;
1206-
await showError(context, errorMessage);
1206+
showError(context, errorMessage);
12071207
}
12081208
}
12091209

lib/widgets/dialog.dart

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,33 @@ Widget _dialogActionText(String text) {
1515
);
1616
}
1717

18-
/// Displays an [AlertDialog] with a dismiss button.
18+
/// A wrapper providing access to the status of an [AlertDialog].
1919
///
20-
/// Returns a [Future] that resolves when the dialog is closed.
21-
Future<void> showErrorDialog({
20+
/// See also:
21+
/// * [showDialog], whose return value this class is intended to wrap.
22+
class DialogStatusController {
23+
const DialogStatusController(this._closed);
24+
25+
final Future<void> _closed;
26+
/// Resolves when the dialog is closed.
27+
Future<void> get closed => _closed;
28+
}
29+
30+
/// Displays an [AlertDialog] with a dismiss button.
31+
///
32+
/// Returns a [DialogStatusController]. Useful for checking if the dialog has
33+
/// been closed.
34+
// This API is inspired by [ScaffoldManager.showSnackBar]. We wrap
35+
// [showDialog]'s return value, a [Future], inside [DialogStatusController]
36+
// whose documentation can be accessed. This helps avoid confusion when
37+
// intepreting the meaning of the [Future].
38+
DialogStatusController showErrorDialog({
2239
required BuildContext context,
2340
required String title,
2441
String? message,
2542
}) {
2643
final zulipLocalizations = ZulipLocalizations.of(context);
27-
return showDialog(
44+
final future = showDialog<void>(
2845
context: context,
2946
builder: (BuildContext context) => AlertDialog(
3047
title: Text(title),
@@ -34,6 +51,7 @@ Future<void> showErrorDialog({
3451
onPressed: () => Navigator.pop(context),
3552
child: _dialogActionText(zulipLocalizations.errorDialogContinue)),
3653
]));
54+
return DialogStatusController(future);
3755
}
3856

3957
void showSuggestedActionDialog({

lib/widgets/lightbox.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,11 +476,12 @@ class _VideoLightboxPageState extends State<VideoLightboxPage> with PerAccountSt
476476
assert(debugLog("VideoPlayerController.initialize failed: $error"));
477477
if (!mounted) return;
478478
final zulipLocalizations = ZulipLocalizations.of(context);
479-
// Wait until the dialog is closed
480-
await showErrorDialog(
479+
final dialog = showErrorDialog(
481480
context: context,
482481
title: zulipLocalizations.errorDialogTitle,
483482
message: zulipLocalizations.errorVideoPlayerFailed);
483+
// Wait until the dialog is closed
484+
await dialog.closed;
484485
if (!mounted) return;
485486
Navigator.pop(context); // Pops the lightbox
486487
}

lib/widgets/login.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ class _LoginPageState extends State<LoginPage> {
307307
if (e is PlatformException && e.message != null) {
308308
message = e.message!;
309309
}
310-
await showErrorDialog(context: context,
310+
showErrorDialog(context: context,
311311
title: zulipLocalizations.errorWebAuthOperationalErrorTitle,
312312
message: message);
313313
} finally {
@@ -351,7 +351,7 @@ class _LoginPageState extends State<LoginPage> {
351351
if (e is PlatformException && e.message != null) {
352352
message = e.message!;
353353
}
354-
await showErrorDialog(context: context,
354+
showErrorDialog(context: context,
355355
title: zulipLocalizations.errorWebAuthOperationalErrorTitle,
356356
message: message);
357357
}

0 commit comments

Comments
 (0)