Introduces DialogCollator

[rjrjr]

Android notoriously doesn't give us any control over the z order of Dialog windows
other than taking care to show() them in the right order, so keeping a pile
of windows in sync with a list of Overlay rendering models is hard.

We've already solved that for individual instances of LayeredDialogSessions
(the delegate that does all of the work for BodyAndOverlaysContainer), but
it was still a problem when you nested them, e.g. in the style of the
recently introduced Nested Overlays sample. Also, the solution was kind of
nasty because we would replace any out of order Dialogs with brand new ones,
rather than preserving them by calling dismiss() and show().

To fix all that we introduce internal class DialogCollator. On each view
update pass, a new DialogCollator instance is created by the outermost
LayeredDialogSession and made available to any nested instances via the
ViewEnvironment. The shared collator collects a list of all existing
DialogSession instances, and a set of DialogSessionUpdate objects to be
asynchronously used to update existing instances or create new ones.

Because a DialogCollator has complete knowledge of all the windows managed
by the set of nested LayerDialogSession instances it supports, it is able to
ensure that inserts are supported by calling dismiss() / show() on existing
windows in the sequence needed to reorder them.

One important change to note: our SavedStateRegistry code requires that each
window in a set has a unique name, which we derive by applying Compatible.keyFor to
its defining Overlay. We used to append to that key an Overlay's position in its
LayeredDialogSessions list, as a poorly considered hack to simplify showing
several of the same type at once. That approach conflicts with reordering.

With this commit we no longer include the index value in SavedStateRegistry keys,
meaning that each Overlay shown by a LayeredDialogSession must have a unique
key -- BackStackScreen has long had a similar requirement. NamedScreen
exists to simplify that kind of thing, and in particular can be used to wrap
ScreenOverlay.content to meet the new requirement.

Fixes #966

[vgonda]

Lol @square/ui-systems-android was literally just talking about how painful dialogs can be and if it would be less painful to draw them manually 🙊

[rjrjr]

Last time we tried that it was just painful in a different way — enough so that we went back to dialogs. But that was a long time ago. And the decoupling provided by the Overlay interface means it will be much easier to try again.

…

On Thu, Mar 30, 2023, 7:04 AM Victoria Gonda ***@***.***> wrote: Lol @square/ui-systems-android <https://github.com/orgs/square/teams/ui-systems-android> was literally just talking about how painful dialogs can be and if it would be less painful to draw them manually 🙊 — Reply to this email directly, view it on GitHub <#981 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOMCHOUCKMUSWQRVTYX4PTW6WHH5ANCNFSM6AAAAAAWMEII2I> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rjrjr]

Moved this back to a draft, Square's internal test suite has issues with it.

[rjrjr]

One vacation and one sick bed later, this has passed our internal test suite with flying colors.

[steve-the-edwards]

Lots of kdoc/understandability questions.

I'm going to have to trust the tests for this to a certain extent as, as you noted, its very complex!

Would be great to have a class diagram and a mock up of how that maps to your main example app used for tests.

But ... maybe there is not need for that as this code will hopefully not be often maintained?

[rjrjr]

Great feedback, thanks @steve-the-edwards.

[steve-the-edwards]

Nice. I understand better now I think! Can you contribute this back to Android 🙏🏻 😆

[steve-the-edwards]

nit: The work here to set up a new session is still a bit hidden after the elvis operator.

I think its fine but a comment might help, "If we don't have an old session to update, create a new session and initialize it to show the dialog in this environment."

[rjrjr]

        oldSessionOrNull
          // If there is already a matching session, it may have been hidden b/c out of order.
          ?.also { session -> session.show(overlay, dialogEnv) }
          // This is a new Overlay, create and show its Dialog.