Skip to content

Commit abacdb6

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: zulip#587 (comment) Signed-off-by: Zixuan James Li <[email protected]>
1 parent c446b0e commit abacdb6

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

0 commit comments

Comments
 (0)