From 12f39ead855690143e4b94a0575ca8875e4cbd4e Mon Sep 17 00:00:00 2001
From: Zixuan James Li <zixuan@zulip.com>
Date: Thu, 12 Sep 2024 00:00:48 -0400
Subject: [PATCH] lightbox: Replace onDismiss with `await showErrorDialog`

The `showDialog` function returns a future that resolves when the
dialog is dismissed, and showErrorDialog returns that future.

The one caller of showErrorDialog that was passing an onDismiss
callback can therefore await that future instead.  Do that, and
eliminate the parameter.

This is not an NFC change, because now the user can tap the barrier
(the darkened area around the dialog) to close it.

Discussion from when this parameter was introduced:
  https://github.com/zulip/zulip-flutter/pull/587#discussion_r1585845428

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
---
 lib/widgets/dialog.dart   | 11 ++++-------
 lib/widgets/lightbox.dart | 20 +++++++++-----------
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart
index 26b07fe267..d4d60128b8 100644
--- a/lib/widgets/dialog.dart
+++ b/lib/widgets/dialog.dart
@@ -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)),
       ]));
 }
diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart
index 8035b9980c..15751bbde1 100644
--- a/lib/widgets/lightbox.dart
+++ b/lib/widgets/lightbox.dart
@@ -474,17 +474,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
     }
   }