Skip to content

Commit 0bae716

Browse files
committed
lightbox [nfc]: 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 <zixuan@zulip.com>
1 parent 34da52f commit 0bae716

File tree

2 files changed

+13
-18
lines changed

2 files changed

+13
-18
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: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -473,17 +473,15 @@ class _VideoLightboxPageState extends State<VideoLightboxPage> with PerAccountSt
473473
await _controller!.play();
474474
} catch (error) { // TODO(log)
475475
assert(debugLog("VideoPlayerController.initialize failed: $error"));
476-
if (mounted) {
477-
final zulipLocalizations = ZulipLocalizations.of(context);
478-
await showErrorDialog(
479-
context: context,
480-
title: zulipLocalizations.errorDialogTitle,
481-
message: zulipLocalizations.errorVideoPlayerFailed,
482-
onDismiss: () {
483-
Navigator.pop(context); // Pops the dialog
484-
Navigator.pop(context); // Pops the lightbox
485-
});
486-
}
476+
if (!mounted) return;
477+
final zulipLocalizations = ZulipLocalizations.of(context);
478+
// Wait until the dialog is closed
479+
await showErrorDialog(
480+
context: context,
481+
title: zulipLocalizations.errorDialogTitle,
482+
message: zulipLocalizations.errorVideoPlayerFailed);
483+
if (!mounted) return;
484+
Navigator.pop(context); // Pops the lightbox
487485
}
488486
}
489487

0 commit comments

Comments
 (0)