Skip to content

Commit 33b97ef

Browse files
committed
dialog [nfc]: Disable barrierDismissible on custom onDismiss
Based on the reasoning at the one place we're using either of these options so far, it seems like we'll always want them coupled this way. (At least until some other reason comes along, and in that case we can revise this logic again.)
1 parent d5ece54 commit 33b97ef

File tree

2 files changed

+8
-10
lines changed

2 files changed

+8
-10
lines changed

lib/widgets/dialog.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,22 @@ Future<void> showErrorDialog({
1919
required BuildContext context,
2020
required String title,
2121
String? message,
22-
bool barrierDismissible = true,
23-
VoidCallback? onContinue,
22+
VoidCallback? onDismiss,
2423
}) {
2524
final zulipLocalizations = ZulipLocalizations.of(context);
2625
return showDialog(
2726
context: context,
28-
barrierDismissible: barrierDismissible,
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,
2932
builder: (BuildContext context) => AlertDialog(
3033
title: Text(title),
3134
content: message != null ? SingleChildScrollView(child: Text(message)) : null,
3235
actions: [
3336
TextButton(
34-
onPressed: onContinue ?? () => Navigator.pop(context),
37+
onPressed: onDismiss ?? () => Navigator.pop(context),
3538
child: _dialogActionText(zulipLocalizations.errorDialogContinue)),
3639
]));
3740
}

lib/widgets/lightbox.dart

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -400,12 +400,7 @@ class _VideoLightboxPageState extends State<VideoLightboxPage> with PerAccountSt
400400
context: context,
401401
title: zulipLocalizations.errorDialogTitle,
402402
message: zulipLocalizations.errorVideoPlayerFailed,
403-
// To avoid showing the disabled video lightbox for the unnsupported
404-
// video, we make sure user doesn't reach there by dismissing the dialog
405-
// by clicking around it, user must press the 'OK' button, which will
406-
// take user back to content message list.
407-
barrierDismissible: false,
408-
onContinue: () {
403+
onDismiss: () {
409404
Navigator.pop(context); // Pops the dialog
410405
Navigator.pop(context); // Pops the lightbox
411406
});

0 commit comments

Comments
 (0)