Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DecorativeViewFactory should support wrapping Context #413

Closed
zach-klippenstein opened this issue Apr 23, 2021 · 10 comments
Closed

DecorativeViewFactory should support wrapping Context #413

zach-klippenstein opened this issue Apr 23, 2021 · 10 comments
Labels
enhancement New feature or request ui Related to UI integration
Milestone

Comments

@zach-klippenstein
Copy link
Collaborator

This is about the only thing it doesn't support, but I need it for wrapping the context at the root.

@zach-klippenstein zach-klippenstein added the enhancement New feature or request label Apr 23, 2021
@rjrjr rjrjr added the ui Related to UI integration label Apr 26, 2021
@rjrjr rjrjr added this to the ui-1.0 milestone Sep 15, 2021
@rjrjr rjrjr self-assigned this Dec 3, 2021
@rjrjr
Copy link
Contributor

rjrjr commented Dec 3, 2021

Also handy just for reading things from the Context, e.g. to populate the ViewEnvironment from it.

@rjrjr
Copy link
Contributor

rjrjr commented Dec 3, 2021

Might be less crazy making to do this with a separate decorator. The tricky bit is that the map function, (OuterT, ViewEnvironment) -> Pair<InnerT, ViewEnvironment>, gets run twice:

  • when we're building the view
  • when we update the view

So we don't want to change the function to:

(OuterT, ViewEnvironment, Context) -> Triple<InnerT, ViewEnvironment, Context>

because the Context transformation is only useful at build time, it would be wasted effort when updating.

@rjrjr
Copy link
Contributor

rjrjr commented Dec 3, 2021

And if a separate wrapper makes sense for Context transformations, maybe ViewEnvironment swizzling should move there too. We hardly ever use it, and when we do it's pretty confusing. Gonna look at current uses…

@rjrjr
Copy link
Contributor

rjrjr commented Dec 3, 2021

Well poo. This actually isn't useful for seeding the ViewEnvironment with values from the Context, which is what made me dust off this issue. There isn't easy enough access to the Context on updates. Putting it back in the icebox.

@rjrjr rjrjr removed their assignment Dec 3, 2021
@rjrjr
Copy link
Contributor

rjrjr commented Jan 7, 2022

This will be addressed by #606

@rjrjr
Copy link
Contributor

rjrjr commented Jan 11, 2022

This will be addressed by #606

Nope, that didn't work out.

@rjrjr
Copy link
Contributor

rjrjr commented Feb 23, 2022

Possible better solution from @helios175:

fun <T : Any> ViewFactory<T>.wrapContext(
  type: KClass<T>,
  wrapContext: (ViewEnvironment, Context) -> Context
): ViewFactory<T> {
  val original = this
  return object : ViewFactory<T> {
    override val type: KClass<in T> = type

    override fun buildView(
      initialRendering: T,
      initialViewEnvironment: ViewEnvironment,
      contextForNewView: Context,
      container: ViewGroup?
    ): View {
      val adjustedContext = wrapContext(initialViewEnvironment, contextForNewView)
      return original.buildView(initialRendering, initialViewEnvironment, adjustedContext, container)
    }
  }
}

@rjrjr
Copy link
Contributor

rjrjr commented Feb 23, 2022

Be nice to replace DecorativewScreenViewFactory w/a set of extensions like this. At least make it non-public API.

@rjrjr
Copy link
Contributor

rjrjr commented Feb 23, 2022

That snippet might be naive about the requirements for LayoutInflater, though. See https://github.com/square/mortar/blob/master/mortar/src/main/java/mortar/MortarContextWrapper.java#L32

rjrjr added a commit that referenced this issue Mar 21, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 21, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 21, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 21, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 21, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 22, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

Reviewers should also note that the **`buildView` method no longer has access to the rendering**, a choice I'm still second guessing. So far I haven't hit a use case that required it, beyond a couple of changes to expected strings in `BackStackContainerPersistenceTest`. Perhaps that's reason enough to bring it back?

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 22, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

Reviewers should also note that the **`buildView` method no longer has access to the rendering**, a choice I'm still second guessing. So far I haven't hit a use case that required it, beyond a couple of changes to expected strings in `BackStackContainerPersistenceTest`. Perhaps that's reason enough to bring it back?

It is also worth calling out that `ScreenViewFactory` still is not involved in the `canShow` path. That was a very conscious choice. Making that call dynamic would add a lot of flexibility, but we've never needed that flexibility. The downside of adding it is that we would lose our 1:1 relationship between factories and rendering types, and suddenly would need a resolution mechanism. The cost benefit analysis there seems clear to me.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 24, 2022
The legacy workflow-ui world is built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This is overly clever, needlessly throws away a good bit of compile time safety, and makes it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` is required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wraps another, with the intention of delegating to its registered `ViewFactory`, that requires creating a wrapper `ViewFactory` that makes a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which is still pretty tricky to use.

To solve all this, we do two things:

- deprecate all the existing `View` extension methods and abandon `WorkflowViewState`, replacing it with `ScreenViewHolder`
- add an `updateView(View)` method to `ScreenViewFactory`

`ScreenViewHolder` is inverted from `WorkflowViewState` -- instead of belonging to the `View`, nestled in a tag, the `View` belongs to it. Most of the old `View` extension methods are replaced by methods on `ScreenViewHolder`. It is very similar to `DialogHolder`, but is part of the public API.

The change to `ScreenViewFactory`, giving it an `update` method, makes it much more like `OverlayDialogFactory`. I toyed a bit with the idea of trying to unify `ScreenViewFactory` / `OverlayDialogFactory` and `ScreenViewHolder` / `DialogHolder`, but it didn't seem worth the added complexity, especially around parameter types.

Reviewers should also note that the **`buildView` method no longer has access to the rendering**, a choice I'm still second guessing. So far I haven't hit a use case that required it, beyond a couple of changes to expected strings in `BackStackContainerPersistenceTest`. Perhaps that's reason enough to bring it back?

It is also worth calling out that `ScreenViewFactory` still is not involved in the `canShow` path. That was a very conscious choice. Making that call dynamic would add a lot of flexibility, but we've never needed that flexibility. The downside of adding it is that we would lose our 1:1 relationship between factories and rendering types, and suddenly would need a resolution mechanism. The cost benefit analysis there seems clear to me.

One of the deprecated `View` extensions did not move to `ScreenViewHolder`: `View.Start` is replaced by `ScreenViewFactory.start`. This extension method is what calls `ScreenViewFactory.buildView`. It creates and returns a `ScreenViewHolder` that wraps the new view and accept updates for it, which it does by calling `ScreenViewFactory.update`, as you'd expect. `ScreenViewFactory.start` is also the new home for the `ViewStarter` mechanism that is used by `WorkflowViewStub` and other containers to call `WorkflowLifecycleOwner.installOn`.

We are not completely out of the `View.setTag` business. `ScreenViewFactory` implementations must be stateless -- it is common for separate instances to field calls to `buildView` and `updateView`, or for a single factory instance to build and update multiple views. `View.setTag` is the most practical way for `buildView` to set up state that `updateView` can rely on. This technique is used as an implementation detail of several standard `ScreenViewFactory` implementations, but is never a public concern. In particular, the factories built by the various `ScreenViewRunner.bind` methods rely on this technique to keep a `View` paired with its runner.

And `ScreenViewRunner` is still very much needed under the new scheme, so that we don't force every implementation of `ScreenViewFactory` to solve this problem of maintaining per-view state themselves. But `ScreenViewRunner` is now a convenience, rather than a fundamental mechanism.

By making `ScreenViewFactory` responsible for both building and updating, it becomes trivial for one factory to delegate to another -- the update method no longer lives in a fragile `View` tag, and so no longer needs to be replaced. A wrapper factory can just delegate to another one. This means there is no longer a problem to be solved by `DecorativeScreenViewFactory`, so that class is deleted. `ManualScreenViewFactory` is also gone, replaced by a `bindBuiltView()` function that returns `Pair<View, ScreenViewRunner>`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 29, 2022
The legacy workflow-ui world was built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This was overly clever, needlessly threw away a good bit of compile time safety, and made it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` was required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wrapped another, with the intention of delegating to its registered `ViewFactory`, that required creating a wrapper `ViewFactory` that made a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which was still pretty tricky to use.

To solve all this, we reverse the relationship between `View` and `WorkflowViewState`, with the latter being replaced by `ScreenViewHolder`. Instead of returing a raw `View`, `ScreenViewFactory.buildView` now returns the view in a `ScreenViewHolder`. The call to `ScreenViewHolder()` replaces the old call to `bindShowRendering`, and the compiler makes it impossible to forget.

`ScreenViewHolder` is a box that holds:

- a `View`
- the `(Screen, ViewEnvironment) -> Unit` function that updates it (still `fun interface ScreenViewRunner` for ease of implementation)
- and the latest `ViewEnvironment` that function received

It is worth noting that `ScreenViewHolder.showing` `.canShow` and `.show()` are all static extensions, built around the new `Showing: ViewEnvironmentKey<Screen>`. `ScreenViewHolder` itself is "write only", with an `in ScreenT: Screen` type parameter -- you can push strongly typed `Screen` renderings into it, but you can't pull them out again.

This minimalist `ScreenViewHolder` interface is trivial to wrap, so all the complexity that inspired `DecorativeViewFactory` is gone. It is replaced by the new `ScreenViewFactory.unwrapping` extension functions, whose use is demontrated in `NamedScreenViewFactory`, `EnvironmentScreenViewFactory` and `BackButtonScreen`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.
rjrjr added a commit that referenced this issue Mar 29, 2022
The legacy workflow-ui world was built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This was overly clever, needlessly threw away a good bit of compile time safety, and made it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` was required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wrapped another, with the intention of delegating to its registered `ViewFactory`, that required creating a wrapper `ViewFactory` that made a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which was still pretty tricky to use.

To solve all this, we reverse the relationship between `View` and `WorkflowViewState`, with the latter being replaced by `ScreenViewHolder`. Instead of returing a raw `View`, `ScreenViewFactory.buildView` now returns the view in a `ScreenViewHolder`. The call to `ScreenViewHolder()` replaces the old call to `bindShowRendering`, and the compiler makes it impossible to forget.

`ScreenViewHolder` is a box that holds:

- a `View`
- the `(Screen, ViewEnvironment) -> Unit` function that updates it (still `fun interface ScreenViewRunner` for ease of implementation)
- and the latest `ViewEnvironment` that function received

It is worth noting that `ScreenViewHolder.showing` `.canShow` and `.show()` are all static extensions, built around the new `Showing: ViewEnvironmentKey<Screen>`. `ScreenViewHolder` itself is "write only", with an `in ScreenT: Screen` type parameter -- you can push strongly typed `Screen` renderings into it, but you can't pull them out again.

This minimalist `ScreenViewHolder` interface is trivial to wrap, so all the complexity that inspired `DecorativeViewFactory` is gone. It is replaced by the new `ScreenViewFactory.unwrapping` extension functions, whose use is demontrated in `NamedScreenViewFactory`, `EnvironmentScreenViewFactory` and `BackButtonScreen`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.

```
title WorkflowViewStub.show()

participant "WorkflowViewStub" as stub
participant Screen
participant "ViewEnvironment" as env
participant "ScreenViewFactoryFinder" as finder
participant "ViewRegistry" as registry
participant "ScreenViewFactory" as factory
participant "ScreenViewHolder" as holder

[->stub:show(screen, env)

stub->Screen:buildView(env)
activate Screen
Screen->Screen:toViewFactory(env)
activate Screen
Screen->env:get(ScreenViewFactoryFinder)
Screen<--env:finder
Screen->finder:getViewFactoryForRendering(this)
activate finder
finder->registry:getEntryFor(screen)
finder<--registry:factory
Screen<--finder:factory
deactivate finder
deactivate Screen

Screen->factory:start(this, env)
activate factory
factory->factory:buildView(screen, env)
activate factory
factory->holder:init(screen, env, runner)
holder-->factory:holder
deactivate factory
factory->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
Screen<--factory:holder
deactivate factory

stub<--Screen:holder
deactivate Screen

[->stub:show(screen, env)
activate stub
stub->holder:canShow(screen)
activate holder
holder->holder: compatible(this.environment[Showing], screen)
stub<--holder:true
deactivate holder
stub->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
deactivate stub
```
rjrjr added a commit that referenced this issue Mar 29, 2022
The legacy workflow-ui world was built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This was overly clever, needlessly threw away a good bit of compile time safety, and made it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` was required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wrapped another, with the intention of delegating to its registered `ViewFactory`, that required creating a wrapper `ViewFactory` that made a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which was still pretty tricky to use.

To solve all this, we reverse the relationship between `View` and `WorkflowViewState`, with the latter being replaced by `ScreenViewHolder`. Instead of returing a raw `View`, `ScreenViewFactory.buildView` now returns the view in a `ScreenViewHolder`. The call to `ScreenViewHolder()` replaces the old call to `bindShowRendering`, and the compiler makes it impossible to forget.

`ScreenViewHolder` is a box that holds:

- a `View`
- the `(Screen, ViewEnvironment) -> Unit` function that updates it (still `fun interface ScreenViewRunner` for ease of implementation)
- and the latest `ViewEnvironment` that function received

It is worth noting that `ScreenViewHolder.showing` `.canShow` and `.show()` are all static extensions, built around the new `Showing: ViewEnvironmentKey<Screen>`. `ScreenViewHolder` itself is "write only", with an `in ScreenT: Screen` type parameter -- you can push strongly typed `Screen` renderings into it, but you can't pull them out again.

This minimalist `ScreenViewHolder` interface is trivial to wrap, so all the complexity that inspired `DecorativeViewFactory` is gone. It is replaced by the new `ScreenViewFactory.unwrapping` extension functions, whose use is demontrated in `NamedScreenViewFactory`, `EnvironmentScreenViewFactory` and `BackButtonScreen`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.

```
title WorkflowViewStub.show()

participant "WorkflowViewStub" as stub
participant Screen
participant "ViewEnvironment" as env
participant "ScreenViewFactoryFinder" as finder
participant "ViewRegistry" as registry
participant "ScreenViewFactory" as factory
participant "ScreenViewHolder" as holder

[->stub:show(screen, env)

stub->Screen:buildView(env)
activate Screen
Screen->Screen:toViewFactory(env)
activate Screen
Screen->env:get(ScreenViewFactoryFinder)
Screen<--env:finder
Screen->finder:getViewFactoryForRendering(this)
activate finder
finder->registry:getEntryFor(screen)
finder<--registry:factory
Screen<--finder:factory
deactivate finder
deactivate Screen

Screen->factory:start(this, env)
activate factory
factory->factory:buildView(screen, env)
activate factory
factory->holder:init(screen, env, runner)
holder-->factory:holder
deactivate factory
factory->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
Screen<--factory:holder
deactivate factory

stub<--Screen:holder
deactivate Screen

[->stub:show(screen, env)
activate stub
stub->holder:canShow(screen)
activate holder
holder->holder: compatible(this.environment[Showing], screen)
stub<--holder:true
deactivate holder
stub->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
deactivate stub
```
rjrjr added a commit that referenced this issue Mar 29, 2022
The legacy workflow-ui world was built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This was overly clever, needlessly threw away a good bit of compile time safety, and made it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` was required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wrapped another, with the intention of delegating to its registered `ViewFactory`, that required creating a wrapper `ViewFactory` that made a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which was still pretty tricky to use.

To solve all this, we reverse the relationship between `View` and `WorkflowViewState`, with the latter being replaced by `ScreenViewHolder`. Instead of returing a raw `View`, `ScreenViewFactory.buildView` now returns the view in a `ScreenViewHolder`. The call to `ScreenViewHolder()` replaces the old call to `bindShowRendering`, and the compiler makes it impossible to forget.

`ScreenViewHolder` is a box that holds:

- a `View`
- the `(Screen, ViewEnvironment) -> Unit` function that updates it (still `fun interface ScreenViewRunner` for ease of implementation)
- and the latest `ViewEnvironment` that function received

It is worth noting that `ScreenViewHolder.showing` `.canShow` and `.show()` are all static extensions, built around the new `Showing: ViewEnvironmentKey<Screen>`. `ScreenViewHolder` itself is "write only", with an `in ScreenT: Screen` type parameter -- you can push strongly typed `Screen` renderings into it, but you can't pull them out again.

This minimalist `ScreenViewHolder` interface is trivial to wrap, so all the complexity that inspired `DecorativeViewFactory` is gone. It is replaced by the new `ScreenViewFactory.unwrapping` extension functions, whose use is demontrated in `NamedScreenViewFactory`, `EnvironmentScreenViewFactory` and `BackButtonScreen`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.

```
title WorkflowViewStub.show()

participant "WorkflowViewStub" as stub
participant Screen
participant "ViewEnvironment" as env
participant "ScreenViewFactoryFinder" as finder
participant "ViewRegistry" as registry
participant "ScreenViewFactory" as factory
participant "ScreenViewHolder" as holder

[->stub:show(screen, env)

stub->Screen:buildView(env)
activate Screen
Screen->Screen:toViewFactory(env)
activate Screen
Screen->env:get(ScreenViewFactoryFinder)
Screen<--env:finder
Screen->finder:getViewFactoryForRendering(this)
activate finder
finder->registry:getEntryFor(screen)
finder<--registry:factory
Screen<--finder:factory
deactivate finder
deactivate Screen

Screen->factory:start(this, env)
activate factory
factory->factory:buildView(screen, env)
activate factory
factory->holder:init(screen, env, runner)
holder-->factory:holder
deactivate factory
factory->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
Screen<--factory:holder
deactivate factory

stub<--Screen:holder
deactivate Screen

[->stub:show(screen, env)
activate stub
stub->holder:canShow(screen)
activate holder
holder->holder: compatible(this.environment[Showing], screen)
stub<--holder:true
deactivate holder
stub->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
deactivate stub
```
rjrjr added a commit that referenced this issue Mar 30, 2022
Replaces #703.

The legacy workflow-ui world was built around a set of extension methods on `View`, backed by a `WorkflowViewState` object hanging off of a tag. This was overly clever, needlessly threw away a good bit of compile time safety, and made it much harder than it should be to implement wrapper renderings.

In particular, every `ViewFactory` was required to call `View.bindShowRendering` to put all this machinery in place. When a screen rendering wrapped another, with the intention of delegating to its registered `ViewFactory`, that required creating a wrapper `ViewFactory` that made a new `bindShowRendering` call replacing and wrapping the `WorkflowViewState` put in place by the delegate. That's so tricky it required the introduction of `DecorativeViewFactory`, which was still pretty tricky to use.

To solve all this, we reverse the relationship between `View` and `WorkflowViewState`, with the latter being replaced by `ScreenViewHolder`. Instead of returing a raw `View`, `ScreenViewFactory.buildView` now returns the view in a `ScreenViewHolder`. The call to `ScreenViewHolder()` replaces the old call to `bindShowRendering`, and the compiler makes it impossible to forget.

`ScreenViewHolder` is a box that holds:

- a `View`
- the `(Screen, ViewEnvironment) -> Unit` function that updates it (still `fun interface ScreenViewRunner` for ease of implementation)
- and the latest `ViewEnvironment` that function received

It is worth noting that `ScreenViewHolder.showing` `.canShow` and `.show()` are all static extensions, built around the new `Showing: ViewEnvironmentKey<Screen>`. `ScreenViewHolder` itself is "write only", with an `in ScreenT: Screen` type parameter -- you can push strongly typed `Screen` renderings into it, but you can't pull them out again.

This minimalist `ScreenViewHolder` interface is trivial to wrap, so all the complexity that inspired `DecorativeViewFactory` is gone. It is replaced by the new `ScreenViewFactory.unwrapping` extension functions, whose use is demontrated in `NamedScreenViewFactory`, `EnvironmentScreenViewFactory` and `BackButtonScreen`.

The `ViewStarter` mechanism is also much simpler now, also no longer relying on any state stored in `View` tags. The implementation of `ScreenViewFactory.start()` remains a bit complex, but only because of the work it has to do to keep the legacy `View.start()` method working, in partnership with `AsScreenViewFactory()`. Once we delete the deprecated View code, most of the code in both of those can be deleted.

Fixes #413, fixes #598, fixes #626, fixes #551, fixes #443.

```
title WorkflowViewStub.show()

participant "WorkflowViewStub" as stub
participant Screen
participant "ViewEnvironment" as env
participant "ScreenViewFactoryFinder" as finder
participant "ViewRegistry" as registry
participant "ScreenViewFactory" as factory
participant "ScreenViewHolder" as holder

[->stub:show(screen, env)

stub->Screen:buildView(env)
activate Screen
Screen->Screen:toViewFactory(env)
activate Screen
Screen->env:get(ScreenViewFactoryFinder)
Screen<--env:finder
Screen->finder:getViewFactoryForRendering(this)
activate finder
finder->registry:getEntryFor(screen)
finder<--registry:factory
Screen<--finder:factory
deactivate finder
deactivate Screen

Screen->factory:start(this, env)
activate factory
factory->factory:buildView(screen, env)
activate factory
factory->holder:init(screen, env, runner)
holder-->factory:holder
deactivate factory
factory->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
Screen<--factory:holder
deactivate factory

stub<--Screen:holder
deactivate Screen

[->stub:show(screen, env)
activate stub
stub->holder:canShow(screen)
activate holder
holder->holder: compatible(this.environment[Showing], screen)
stub<--holder:true
deactivate holder
stub->holder:show(screen, env)
activate holder
holder->holder:this.environment = env + (Showing to screen)
holder->holder:this.runner.show(screen, environment)
deactivate holder
deactivate stub
```
@rjrjr
Copy link
Contributor

rjrjr commented Apr 15, 2022

ScreenViewFactory supports that easily.

@rjrjr rjrjr closed this as completed Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ui Related to UI integration
Projects
None yet
Development

No branches or pull requests

2 participants