Introduces ScreenOverlay and ModalScreenOverlayDialogFactory

[rjrjr]

ScreenOverlay marks an Overlay with a Screen defining its content.

ModalScreenOverlayDialogFactory can show a ScreenOverlay as an
android.app.Dialog, taking care of updating the content view and managing
the Back button. It's the much simpler replacement for ModalViewContainer.

ModalScreenOverlayDialogFactory works with BodyAndModalsContainer
and LayeredDialogs to perform two important tricks:

• updateBounds(Dialog, Rect), which allows dialogs to be restrictred to a subset
  of the screen. Implemented via the private ModalArea value in ViewEnvironment

• Touch and keyboard events are blocked by views that are covered by
  dialogs managed by a ModalScreenOverlayDialogFactory. Same for
  dialogs with a lower z order. We use another private ViewEnvironment
  value for that, CoveredByModal.

In sample code, PanelOverlay replaces PanelContainerScreen, and the Tic
Tac Toe sample is updated to use the new hotness. I think the diff of the
sample code really highlights how much the new marker interfaces improve our
composition story.

That said, the ScrimScreen bit in the sample is a little rough. In the old
code we magically swizzled the scrim into place automatically. This PR
seems big enough already, so I'll follow up with a clean up that does
something similar and deletes all the deprecated sample code.

Fixes #259, #138, #99, #204, #314, #589

[rjrjr]

That said, the ScrimScreen bit in the sample is a little rough. In the old
code we magically swizzled the scrim into place automatically, and I think in
prod I would still do the same, but the sample is complex enough already.

I think I actually need to address that in this PR. Panel is a pretty high fidelity
sample, used all over the place. Now is the time to figure out how to make it usable.

[rjrjr]

Or maybe the thing to do is undelete the old PanelContainer and get this merged, since the PR is already too big. Then follow up with updating all the other samples that use the old modal stuff.

[rjrjr]

Green enough, failure is due to ##590.

[rjrjr]

Update deletes some more unused stuff.

[steve-the-edwards]

😵 . Is this a common thing we do? Could use a why comment for me at least.

Also could the val be maybeWindowBackgroundColour not typedValue?

[rjrjr]

Sample code, so don't worry too much. And I'm afraid it's pretty common Android boilerplate -- I'd even argue idiomatic. Compose can't come fast enough.

But yes, that's a better name, and a comment can help. Not inclined to write a dissertation, though, b/c it's just how Android life is.

[rjrjr]

// Welcome to Android. Nothing workflow-related here, this is just how one
// finds the window background color for the theme. I sure hope it's better in Compose.
val maybeWindowColor = TypedValue()
context.theme.resolveAttribute(android.R.attr.windowBackground, maybeWindowColor, true)
if (maybeWindowColor.type in TypedValue.TYPE_FIRST_COLOR_INT..TypedValue.TYPE_LAST_COLOR_INT) {
  dialog.window!!.setBackgroundDrawable(ColorDrawable(maybeWindowColor.data))
}

[steve-the-edwards]

❤️

[steve-the-edwards]

I don't think we should use the job and then leak the scope until callbacks are cleared off the window. Was there a reason you went this way?

Why not just create the scope onAttach and use that?

[rjrjr]

Because I'm still really bad at Coroutines. Thanks.

Note that we use the same technique in WorkflowLayout.takeWhileAttached, will fix that too.

[rjrjr]

Does this look right?

  window.callback = object : Window.Callback by window.callback {
    var scope : CoroutineScope? = null

    override fun onAttachedToWindow() {
      scope  = CoroutineScope(Dispatchers.Main.immediate).also {
        bounds.onEach { b -> onBoundsChange(this@maintainBounds, b) }
          .launchIn(it)
      }
    }

    override fun onDetachedFromWindow() {
      scope?.cancel()
      scope = null
    }
  }

[steve-the-edwards]

here we are just extracting the 'Global Visible Rect' right? - what does that represent?

[rjrjr]

It's the bounds of the view in global coordinates -- the screen's coordinate system, not the parent view's. But I just noticed that this method returns false "If the view is completely clipped or translated out". I imagine the contents of the Rect are nonsense in that case.

Not even sure what to do if that happens. Probably should close all the dialogs, treat it like onDetach

[steve-the-edwards]

Ok. Sounds good.

[rjrjr]

  // The bounds of this view in global (display) coordinates, as reported
  // by getGlobalVisibleRect.
  //
  // Made available to managed ModalScreenOverlayDialogFactory instances
  // via the ModalArea key in ViewEnvironment. When this updates their
  // updateBounds methods will fire. They should resize themselves to
  // avoid covering peers of this view.
  private val bounds = MutableStateFlow(Rect())
  private val boundsRect = Rect()

  private val boundsListener = OnGlobalLayoutListener {
    if (getGlobalVisibleRect(boundsRect) && boundsRect != bounds.value) {
      bounds.value = Rect(boundsRect)
    }
    // Should we close the dialogs if getGlobalVisibleRect returns false?
    // https://github.com/square/workflow-kotlin/issues/599
  }

[rjrjr]

Note that I punted handling a false return in any interesting way. We've been shipping the equivalent code for more than a year and it's not been an issue. I'd like to dig into that case separately and carefully -- see if it's actually an issue at all.

[rjrjr]

Updated to honor the feedback from @steve-the-edwards, especially around eliminating Bounds and making the two new ViewEnvironment.Keys private.

Also notice the updated commit message, and improved kdoc on updateBounds.

PTAL

[rjrjr]

@steve-the-edwards Oops, I was too quick to ask PTAL, I missed some of your comments. Will ping again soon.

[rjrjr]

@steve-the-edwards Oops, I was too quick to ask PTAL, I missed some of your comments. Will ping again soon.

Actually, okay to review. Tracking the most interesting thing I missed separately, #600.

[steve-the-edwards]

Awesome!

[steve-the-edwards]

I guess I was suggesting to name the arg here still

Suggested change

	RunGameRendering(takeTurnsScreen)
	RunGameRendering(gameScreen = takeTurnsScreen)

but its not a big deal.

[steve-the-edwards]

Ahh - now I understand. This is very helpful.

[steve-the-edwards]

👍🏻 .

Other than the injected immediate main dispatcher - but I see thats covered by #600 .

If we want debugging help we could add a name into the context for the scope.

Suggested change

	window.callback = object : Window.Callback by window.callback {
	var scope: CoroutineScope? = null
	override fun onAttachedToWindow() {
	scope = CoroutineScope(Dispatchers.Main.immediate).also {
	bounds.onEach { b -> onBoundsChange(this@maintainBounds, b) }
	.launchIn(it)
	}
	}
	override fun onDetachedFromWindow() {
	scope?.cancel()
	scope = null
	}
	}
	window.callback = object : Window.Callback by window.callback {
	var scope: CoroutineScope? = null
	override fun onAttachedToWindow() {
	val id = window.decorView.id
	scope = CoroutineScope(Dispatchers.Main.immediate + CoroutineName(id)).also {
	bounds.onEach { b -> onBoundsChange(this@maintainBounds, b) }
	.launchIn(it)
	}
	}
	override fun onDetachedFromWindow() {
	scope?.cancel()
	scope = null
	}
	}

[rjrjr]

I like the CoroutineName() idea a lot. I think we can do better than the id, though, those things are so opaque. Remember that I snuck the content view into a R.id.workflow_modal_dialog_content on the DecorView. If that's present I can use the classname of its rendering, say. And if it's not, maybe the class name of the dialog itself.

[rjrjr]

s/classname/toString()/, 10x more useful. I was shying away from that since it can be big, and that burned us in workflowAction {}. But I think that's okay for infrequent events like creating a dialog.

[rjrjr]

Meh, I'm over engineering this.

Gonna merge as is and add a note to #600 to consider useful scope name at the same time.