Wrapper renderings introduce an extra AndroidView unnecessarily when the wrapped rendering's factory is a ComposeScreenViewFactory. #546
Comments
|
This is still a problem. I've tried and failed to find a general wrapping pattern a number of times, and I'm not confident a practical solution will emerge. A lot of wrapping is done for very view-system-specific purposes, like manipulating View instances and messing with Context. We'd need to come up with a two pass system that allows platform neutral code to manipulate just the structure and ViewEnvironment, and another that actually builds a View or provides a Composable. In the meantime, I think we need to have the workflow compose module provide an alternative |
rjrjr
changed the title
|
|
|
@helios175 has done something similar to this in a one-off. |
|
We're lucky enough to have almost 100% Compose UI so I'm planning to tackle this internally, and maybe just do this:
@rjrjr or @helios175, any new thoughts or pointers would be very much appreciated. |
|
I'm starting a spike on this today, hoping to apply lessons learned from #874 without rewriting the entire world. Basic idea is:
I think this should allow us to provide both Classic and Compose flavors for whatever renderings we want. Something will have to change about Perhaps |
As much as I like that notion, it likely means "productionize #874", and I don't know that I can land that in a reasonable amount of time. |
Less clean but maybe a smaller delta: interface ViewRegistry {
data class Key<RenderingT, FactoryT>(
val renderingType: KClass<RenderingT>,
val factoryType: KClass<FactoryT>
)
interface Entry<RenderingT: Any, FactoryT: Any> {
val key: Key
}
val keys: Set<Key<*, *>>
fun <R: Any, F: Any> getEntryFor(key: Key<R, F>): Entry<R, F>?
} |
|
Actually…holding off on this until we can delete the legacy stuff like |
|
Working on this again, looks doable. |
rjrjr
added a commit
that referenced
this issue
Dec 18, 2023
Paves the way for parallel Classic and Compose implementations of wrappers like `NamedScreen`, to fix #546.
rjrjr
added a commit
that referenced
this issue
Dec 20, 2023
…ing type Paves the way for parallel Classic and Compose implementations of wrappers like `NamedScreen`, to fix #546. Ironically, breaks Compose support in the process, but that is fixed by the next commit.
rjrjr
added a commit
that referenced
this issue
Dec 20, 2023
…actory` `ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support. Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations. Also introduces: - `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()` - `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546. - `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`. Fixes #546
rjrjr
added a commit
that referenced
this issue
Dec 21, 2023
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 9, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 20, 2024
…ing type Paves the way for parallel Classic and Compose implementations of wrappers like `NamedScreen`, to fix #546. Ironically, breaks Compose support in the process, but that is fixed by the next commit.
rjrjr
added a commit
that referenced
this issue
Jan 20, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 20, 2024
…ing type Paves the way for parallel Classic and Compose implementations of wrappers like `NamedScreen`, to fix #546. Ironically, breaks Compose support in the process, but that is fixed by the next commit.
rjrjr
added a commit
that referenced
this issue
Jan 20, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 20, 2024
…ing type Paves the way for parallel Classic and Compose implementations of wrappers like `NamedScreen`, to fix #546. Ironically, breaks Compose support in the process, but that is fixed by the next commit.
rjrjr
added a commit
that referenced
this issue
Jan 20, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 24, 2024
…ing type Paves the way for parallel Classic and Compose implementations of wrappers like `NamedScreen`, to fix #546. Ironically, breaks Compose support in the process, but that is fixed by the next commit.
rjrjr
added a commit
that referenced
this issue
Jan 24, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 24, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key`, which allows renderings to be bound to multiple UI factories, to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key` (which allows renderings to be bound to multiple UI factories) to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key` (which allows renderings to be bound to multiple UI factories) to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key` (which allows renderings to be bound to multiple UI factories) to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
rjrjr
added a commit
that referenced
this issue
Jan 25, 2024
…actory`
This commit takes advantage of the new `ViewRegistry.Key` (which allows renderings to be bound to multiple UI factories) to fix a long standing problem where wrapper screens -- things like `NamedScreen` and `EnvironmentScreen` -- used in a Compose context would cause needless calls to `@Composeable fun AndroidView()` and `ComposeView()`.
For example, consider this rendering:
```
BodyAndOverlaysScreen(
body = SomeComposeScreen(
EnvironmentScreen(
SomeOtherComposeScreen
)
)
)
```
Before this change, that would create a View hierarchy something like this:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
AndroidView {
ComposeView {
// nested compose land
SomeOtherComposeScreen.Content()
```
Now it will look this way:
```
BodyAndOverlaysContainer : FrameLayout {
mChildren[0] = ComposeView {
// compose land
SomeComposeScreen.Content {
SomeOtherComposeScreen.Content()
```
`ScreenComposableFactory` replaces `ComposeScreenViewFactory`, and `ComposeScreen` no longer extends `AndroidScreen`. Compose support is now a first class citizen, instead of a hack bolted on to View support.
Unfortunately, `ViewEnvironment.withComposeInteropSupport()` (see below) now must be called near the root of a Workflow app to enable the seamless Compose > Classic > Compose handling that used to be built in to `WorkflowRendering` and `ComposeScreenViewFactory`. This means that call is required for Compose support for built in rendering types like `BodyAndOverlaysScreen` and `BackStackScreen`, which so far are backed only by classic View implementations.
Other introductions, changes:
- `Screen.toComposableFactory()`, used by `WorkflowRendering()` in the same way that `WorkflowViewStub` uses `Screen.toViewFactory()`
- `ScreenComposableFactoryFinder`, a `ViewEnvironment`-based strategy object used by `Screen.toComposableFactory()` the same way that `Screen.toViewFactory()` uses `ScreenViewFactoryFinder`. The default implementation provides Compose bindings for `NamedScreen` and `EnvironmentScreen`, fixing #546.
- `ScreenViewFactoryFinder.getViewFactoryForRendering()` can now return `null`. A `requireViewFactoryForRendering()` extension is introduced for use when `null` is not acceptable.
- `ViewEnvironment.withComposeInteropSupport()`, which wraps the found `ScreenComposableFactoryFinder` and `ScreenViewFactoryFinder` with implementations that allow Compose contexts to handle renderings bound only to `ScreenViewFactory`, and classic contexts to handle renderings bound only to `ScreenComposableFactory`. Replaces the logic that used to be in the private `ScreenViewFactory.asComposeViewFactory()` extension in `WorkflowRendering()`.
- `Screen.Preview()` is introduced. The existing `Preview()` extension functions were tied to `ScreenViewFactory`, making them much less useful. It is still the case that previews work for non-Compose UI code just fine. Which is pretty cool, really.
Fixes #546
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I just realized this wrapping pattern (even before this change) might introduce an extra view unnecessarily when the wrapped rendering's factory is a
ComposeViewFactory. It shouldn't affect correctness but something we should probably check/test and to keep in mind as yall adopt compose.Originally posted by @zach-klippenstein in #544 (comment)
I think this could probably be addressed if you came up with a way to generalize the wrapping pattern (as @rjrjr suggested on slack), since
WorkflowRendering'sasComposeViewFactorycould use that to do the type check at the leaf.The text was updated successfully, but these errors were encountered: