Skip to content

Commit 7d18cfa

Browse files
committed
lightbox: Replace onDismiss with await showErrorDialog
`showDialog` returns a `Future` that resolves when the dialog is dismissed. This refactors `showErrorDialog` to stop overriding `onPressed` and `barrierIsDismissable`, and make the caller call Navigator.pop one more time once the dialog is closed. Note that we check for `mounted` again because there has been an asynchronous gap. See also: #587 (comment) Signed-off-by: Zixuan James Li <[email protected]>
1 parent 34da52f commit 7d18cfa

File tree

2 files changed

+10
-12
lines changed

2 files changed

+10
-12
lines changed

lib/widgets/dialog.dart

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

18+
/// Displays an [AlertDialog] with a dismiss button.
19+
///
20+
/// Returns a [Future] that resolves when the dialog is closed.
1821
Future<void> showErrorDialog({
1922
required BuildContext context,
2023
required String title,
2124
String? message,
22-
VoidCallback? onDismiss,
2325
}) {
2426
final zulipLocalizations = ZulipLocalizations.of(context);
2527
return showDialog(
2628
context: context,
27-
// `showDialog` doesn't take an `onDismiss`, so dismissing via the barrier
28-
// always causes the default dismiss behavior of popping just this route.
29-
// When we want a non-default `onDismiss`, disable that.
30-
// TODO(upstream): add onDismiss to showDialog, passing through to [ModalBarrier.onDismiss]
31-
barrierDismissible: onDismiss == null,
3229
builder: (BuildContext context) => AlertDialog(
3330
title: Text(title),
3431
content: message != null ? SingleChildScrollView(child: Text(message)) : null,
3532
actions: [
3633
TextButton(
37-
onPressed: onDismiss ?? () => Navigator.pop(context),
34+
onPressed: () => Navigator.pop(context),
3835
child: _dialogActionText(zulipLocalizations.errorDialogContinue)),
3936
]));
4037
}

lib/widgets/lightbox.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -475,14 +475,15 @@ class _VideoLightboxPageState extends State<VideoLightboxPage> with PerAccountSt
475475
assert(debugLog("VideoPlayerController.initialize failed: $error"));
476476
if (mounted) {
477477
final zulipLocalizations = ZulipLocalizations.of(context);
478+
// Wait until the dialog is closed
478479
await showErrorDialog(
479480
context: context,
480481
title: zulipLocalizations.errorDialogTitle,
481-
message: zulipLocalizations.errorVideoPlayerFailed,
482-
onDismiss: () {
483-
Navigator.pop(context); // Pops the dialog
484-
Navigator.pop(context); // Pops the lightbox
485-
});
482+
message: zulipLocalizations.errorVideoPlayerFailed);
483+
if (mounted) {
484+
// Pop the lightbox
485+
Navigator.of(context).pop();
486+
}
486487
}
487488
}
488489
}

0 commit comments

Comments
 (0)