lightbox: Replace onDismiss with await showErrorDialog

[PIG208]

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:
#587 (comment)

[chrisbobbe]

LGTM assuming you've tested this manually; marking for Greg's review.

[PIG208]

I forgot to mention that this shouldn't be an nfc change, because the user can close the dialog by tapping the barrier now. Pushed an update to the commit message to reflect that.

[chrisbobbe]

Makes sense. Ah and I think barrierIsDismissable in the commit message is a typo for barrierDismissable.

[gnprice]

Thanks! This is a nice cleanup.

Merging, with a revision to the commit message:

     lightbox: Replace onDismiss with `await showErrorDialog`
 
-    `showDialog` returns a `Future` that resolves when the dialog is
-    dismissed.
+    The `showDialog` function returns a future that resolves when the
+    dialog is dismissed, and showErrorDialog returns that future.
 
-    This refactors `showErrorDialog` to stop overriding `onPressed` and
-    `barrierDismissible`, and make the caller call `Navigator.pop` one
-    more time once the dialog is closed.
+    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 because now the user can tap the barrier (darkened
-    area around the dialog) to close it.
+    This is not an NFC change, because now the user can tap the barrier
+    (the darkened area around the dialog) to close it.
 
-    Note that we check for `mounted` again because there has been an
-    asynchronous gap.
-
-    See also:
+    Discussion from when this parameter was introduced:
       https://github.com/zulip/zulip-flutter/pull/587#discussion_r1585845428

The main change is to explain more of the "why", and delete several details of "what" which the reader can more easily learn by looking at the diff.

[gnprice]

This behavior (which showDialog has and we faithfully wrap) is kind of a bad pattern for an API, as we've discussed in the context of #731 / the unawaited_futures lint. If a function is named "show thing" and its return type is some Future<T>, the future should complete when the thing has been shown, not when some possibly-much-later event happens.

I think a good model here is ScaffoldMessengerState.showSnackBar: there's a Future it wants to provide for callers to potentially await on, but it's for something that happens after the function's own work is done, so instead of returning a value that is a Future it returns a value that has a Future. And the property name, closed, makes it clear what event is being awaited.

Anyway, that's a problem for outside this PR.