Skip to content

Commit 1df1035

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 `barrierDismissible`, and make the caller call `Navigator.pop` one more time once the dialog is closed. This is not an nfc because now the user can tap the barrier (darkened area around the dialog) to close it. 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 1df1035

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)