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.

Sign up for GitHub

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

Jump to bottom

Replaces *.initializeView with *.viewStarter #602

Merged
merged 2 commits into from
Dec 10, 2021
Merged

Replaces *.initializeView with *.viewStarter #602

merged 2 commits into from
Dec 10, 2021

Commits on Dec 10, 2021

  1. Adds (failing) DecorativeViewFactory double update test

    @rjrjr
    rjrjr committed Dec 10, 2021
    Configuration menu
    Copy the full SHA
    e50052d View commit details
    Browse the repository at this point in the history

  2. Replaces *.initializeView with *.viewStarter 

    Fixes #597.

It used to be the case that every `DecorativeViewFactory` called
`showRendering()` twice (#397). We fixed that (we thought) by introducing the
`initializeView` lambda to `ViewRegistry.buildView` and
`DecorativeViewFactory` (#408).

Unfortunately, that fix botched recursion. Individual `DecorativeViewFactory`
instances work fine, but if you wrap them you still get one `showRendering`
call from each. Worse, upstream `initializeView` lambdas are clobbered by
immediately downstream ones. e.g., when a `WorkflowViewStub` shows a
`DecorativeViewFactory`, the `WorkflowLifecycleRunner.installOn` call in the
former is clobbered.

The fix is to completely decouple building a view from from this kind of
initialization. `ViewRegistry.buildView` and its wrappers no longer try to
call `showRendering` at all.

Instead the caller of `buildView` (mostly `WorkflowViewStub`) is reponsible
for immediately calling `View.start` on the new `View`. `View.start` makes
the initial `showRendering` call that formerly was the job of
`ViewFactory.buildView` -- the factory builds the view, and the container
turns the key. Since `View.start` is called only after all wrapped
`ViewFactory.buildView` functions have executed, we're certain it will only
happen once.

Of course we still need the ability to customize view initialization via
wrapping, especially to invoke `WorkflowLifecycleOwner.installOn`. To
accomodate that, the function that `View.start` executes can be wrapped via
the new `viewStarter` argument to `ViewRegistry.buildView` and
`DecorativeViewFactory`, which replaces `initializeView`.

This required a pretty thorough overhaul of `ViewShowRendering.kt`
The `ViewShowRenderingTag` that it hangs off of a view tag is renamed
`WorkflowViewState`, and extracted to a separate file.

`WorkflowViewState` is a sealed class with two implementations (`New` and
`Started`) to help us enforce the order of the `ViewRegistry.buildView`,
`View.bindShowRendering`, `View.start` and `View.showRendering` calls.
    @rjrjr
    rjrjr committed Dec 10, 2021
    Configuration menu
    Copy the full SHA
    b93a522 View commit details
    Browse the repository at this point in the history