Redundant calls to View.showRendering #397
Comments
|
I agree this is a problem, I'm not sure I agree with your proposed solution. The root of the issue is the logic in Your solution might actually be simpler, but we should consider the tradeoffs. |
|
I think this is a "why not both" situation. Note to self, here's the previous attempt: #276 |
|
Really, |
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Closes #397
rjrjr
added a commit
that referenced
this issue
Apr 16, 2021
That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Closes #397
rjrjr
added a commit
that referenced
this issue
Dec 9, 2021
Fixes #597. It used to be the case that every `DecorativeViewFactory` called `showRendering()` twice (#397). We fixed that 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. The `initializeView` lambdas are gone, and so is `View.showFirstRendering`. Instead the caller of `buildView` (mostly `WorkflowViewStub`) is reponsible for calling a new method, `View.start`, before it makes any calls to `View.showRendering`. `View.start` takes over the `WorkflowLifecycleRunner.installOn` job that was previously done by `WorkflowViewStub` and a few others. Since it is called only after any `ViewFactory` wrapping of the rendering in hand has been peformed, we're certain it will only happen once. Of course we still need the ability to to customize view initialization via wrapping. To accomodate that, `View.start` is implemented by executing another new extension, `internal var View.starter: (View) -> Unit`. Because it is a `var`, `buildView` callers can wrap it before `start` is invoked. We provide a bit of hand holding to those who need this ability to wrap the starter via `DecorativeViewFactory.onStartWrapper`, which replaceds `initializeView`. Where `initializeView` lambdas were expected to make their own call to the static `View.showFirstRendering` function, `onStartWrapper` is passed a function to invoke -- the current contents of `View.starter`. We wrap and replace `var View.starter` for them, and keep it private to the library. This required a pretty thorough overhaul of `ViewShowRendering.kt`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 9, 2021
Fixes #597. It used to be the case that every `DecorativeViewFactory` called `showRendering()` twice (#397). We fixed that 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. The `initializeView` lambdas are gone, and so is `View.showFirstRendering`. Instead the caller of `buildView` (mostly `WorkflowViewStub`) is reponsible for calling a new method, `View.start`, before it makes any calls to `View.showRendering`. `View.start` takes over the `WorkflowLifecycleRunner.installOn` job that was previously done by `WorkflowViewStub` and a few others. Since it is called only after any `ViewFactory` wrapping of the rendering in hand has been peformed, we're certain it will only happen once. Of course we still need the ability to to customize view initialization via wrapping. To accomodate that, `View.start` is implemented by executing another new extension, `internal var View.starter: (View) -> Unit`. Because it is a `var`, `buildView` callers can wrap it before `start` is invoked. We provide a bit of hand holding to those who need this ability to wrap the starter via `DecorativeViewFactory.onStartWrapper`, which replaceds `initializeView`. Where `initializeView` lambdas were expected to make their own call to the static `View.showFirstRendering` function, `onStartWrapper` is passed a function to invoke -- the current contents of `View.starter`. We wrap and replace `var View.starter` for them, and keep it private to the library. This required a pretty thorough overhaul of `ViewShowRendering.kt`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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 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`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, 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
added a commit
that referenced
this issue
Dec 10, 2021
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
added a commit
that referenced
this issue
Dec 10, 2021
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
DecorativeViewFactorymakes redundant calls toshowRendering, and these can compound. I think the proper fix is to factor arebindShowRenderingcall out ofbindShowRenderingthat replaces the function but doesn't call it.The text was updated successfully, but these errors were encountered: