Skip to content

Commit

Permalink
lightbox: Replace onDismiss with await showErrorDialog
Browse files Browse the repository at this point in the history
`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]>
  • Loading branch information
PIG208 committed Sep 19, 2024
1 parent 34da52f commit f27b62c
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 18 deletions.
11 changes: 4 additions & 7 deletions lib/widgets/dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,23 @@ Widget _dialogActionText(String text) {
);
}

/// Displays an [AlertDialog] with a dismiss button.
///
/// Returns a [Future] that resolves when the dialog is closed.
Future<void> showErrorDialog({
required BuildContext context,
required String title,
String? message,
VoidCallback? onDismiss,
}) {
final zulipLocalizations = ZulipLocalizations.of(context);
return showDialog(
context: context,
// `showDialog` doesn't take an `onDismiss`, so dismissing via the barrier
// always causes the default dismiss behavior of popping just this route.
// When we want a non-default `onDismiss`, disable that.
// TODO(upstream): add onDismiss to showDialog, passing through to [ModalBarrier.onDismiss]
barrierDismissible: onDismiss == null,
builder: (BuildContext context) => AlertDialog(
title: Text(title),
content: message != null ? SingleChildScrollView(child: Text(message)) : null,
actions: [
TextButton(
onPressed: onDismiss ?? () => Navigator.pop(context),
onPressed: () => Navigator.pop(context),
child: _dialogActionText(zulipLocalizations.errorDialogContinue)),
]));
}
Expand Down
20 changes: 9 additions & 11 deletions lib/widgets/lightbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -473,17 +473,15 @@ class _VideoLightboxPageState extends State<VideoLightboxPage> with PerAccountSt
await _controller!.play();
} catch (error) { // TODO(log)
assert(debugLog("VideoPlayerController.initialize failed: $error"));
if (mounted) {
final zulipLocalizations = ZulipLocalizations.of(context);
await showErrorDialog(
context: context,
title: zulipLocalizations.errorDialogTitle,
message: zulipLocalizations.errorVideoPlayerFailed,
onDismiss: () {
Navigator.pop(context); // Pops the dialog
Navigator.pop(context); // Pops the lightbox
});
}
if (!mounted) return;
final zulipLocalizations = ZulipLocalizations.of(context);
// Wait until the dialog is closed
await showErrorDialog(
context: context,
title: zulipLocalizations.errorDialogTitle,
message: zulipLocalizations.errorVideoPlayerFailed);
if (!mounted) return;
Navigator.pop(context); // Pops the lightbox
}
}

Expand Down

0 comments on commit f27b62c

Please sign in to comment.