Make ViewFactory.showRendering function responsible for applying the ComposeViewFactoryRoot. #24
Conversation
| viewEnvironment: ViewEnvironment, | ||
| content: @Composable() () -> Unit | ||
| ) { | ||
| if (HasViewFactoryRootBeenApplied.current) { |
There was a problem hiding this comment.
I'm missing where this gets set to true.
There was a problem hiding this comment.
Great question, I'll add a comment to the source. It gets "set" to true on line 79 below. This function can appear multiple times, at multiple nesting levels, inside the composition. The ambient has a default value of false, which means if this function is the top-most one in its subtree, it will read the value as false and drop down to the else case. That case will wrap the child Composable with this ambient providing the value true, so any nested occurrences of this function below this point will see that true value and not re-apply the wrapper.
There was a problem hiding this comment.
God, it was staring me right in the face.
There was a problem hiding this comment.
It's weird to look at if you're not thinking about recursion. I added a bunch of comments that I hope should clear this up.
zach-klippenstein
changed the base branch from
zachklipp/cleanup-packages
to
master
|
I have barely scratched the surface. I've never thought about writing unit tests for UI, but it's so easy. Just wish I could run them without an emulator (might be possible? haven't tried). |
zach-klippenstein
force-pushed
the
zachklipp/better-root
branch
from
8e70be8 to
31407fc
Compare
|
I don't really like that this root thing requires custom logic in the internals. I have an idea for decoupling these pieces in another branch, but it needs some more work. |
…ComposeViewFactoryRoot. This ensures that the root will be applied in all code paths: - Entering the Compose world through a `WorkflowViewStub`/`ComposeViewFactory`. - Already in Compose, showing a rendering from a `WorkflowContainer`. This change also ensures that if the `ComposeViewFactoryRoot` is changed above where it's applied, the new wrapper will be applied. Adds tests for this and other logic. Fixes #20.
zach-klippenstein
force-pushed
the
zachklipp/better-root
branch
from
31407fc to
acfbee0
Compare
| @@ -97,7 +97,7 @@ jobs: | |||
| name: Instrumentation tests | |||
| needs: assemble | |||
| runs-on: macos-latest | |||
| timeout-minutes: 20 | |||
| timeout-minutes: 30 | |||
There was a problem hiding this comment.
This PR adds a bunch of UI tests, so we need to increase the CI timeout a bit.
There was a problem hiding this comment.
Better fix coming in #30.
This ensures that the root will be applied in all code paths:
WorkflowViewStub/ComposeViewFactory.WorkflowContainer.This change also ensures that if the
ComposeViewFactoryRootis changed above where it's applied,the new wrapper will be applied. Adds tests for this and other logic.
Fixes #20.