Overhaul of Compose support. #689
Conversation
rjrjr
force-pushed
the
ray/simpler-compose-overhaul
branch
9 times, most recently
from
5394b1b to
a6b9ecf
Compare
...mpose-tooling/src/main/java/com/squareup/workflow1/ui/compose/tooling/ScreenViewFactories.kt
Outdated
Show resolved
Hide resolved
rjrjr
force-pushed
the
ray/simpler-compose-overhaul
branch
from
a6b9ecf to
8623624
Compare
I had originally thought about creating a new `ScreenComposableFactory : ViewRegistry.Entry`, but that did nothing but introduce complexity. The existing system has the elegant property that every `ViewFactory` can produce either a `@Composable` or a `View` as needed. That's a crucial trait, and it's most easily maintained by teaching the same trick to `ScreenViewFactory`. Where possible, existing tests are copied into new files with a `Legacy` prefix and otherwise unchanged, so that diffs of the original tests will be reviewable.
rjrjr
force-pushed
the
ray/simpler-compose-overhaul
branch
from
8623624 to
e5b5ba9
Compare
|
Update eliminates unnecessary "FunctionName" warning suppression -- my IDE config was botched. |
samples/compose-samples/src/main/java/com/squareup/sample/compose/preview/PreviewActivity.kt
Show resolved
Hide resolved
| * [placeholderViewFactory] that will be used to show any renderings that don't match | ||
| * [mainFactory]'s type. All placeholders will have [placeholderModifier] applied. | ||
| */ | ||
| @Composable internal fun previewViewEnvironment( |
There was a problem hiding this comment.
nit: PreviewViewEnvironment
The same goes for the original/overloaded function above this.
There was a problem hiding this comment.
I think I'll fix that on main.
There was a problem hiding this comment.
Oh, nevermind, it's non-public. Might as well do it here.
There was a problem hiding this comment.
Ah! Nevermind then! My mistake!
There was a problem hiding this comment.
Should be called rememberPreview…
| * @param viewEnvironment The [ViewEnvironment] passed down through the `ViewBinding` pipeline. | ||
| */ | ||
| @Composable public abstract fun RenderingContent( | ||
| @Composable abstract fun RenderingContent( |
There was a problem hiding this comment.
Oops! Good catch, thanks.
There was a problem hiding this comment.
Oh, because this is a sample, no explicit API mode -- public is redundant.
| * | ||
| * *Note: [rendering] must be the same type as this [ViewFactory], even though the type system does | ||
| * not enforce this constraint. This is due to a Compose compiler bug tracked | ||
| * [here](https://issuetracker.google.com/issues/156527332).* |
There was a problem hiding this comment.
I wonder if this is fixed…
| * [placeholderViewFactory] that will be used to show any renderings that don't match | ||
| * [mainFactory]'s type. All placeholders will have [placeholderModifier] applied. | ||
| */ | ||
| @Composable internal fun previewViewEnvironment( |
There was a problem hiding this comment.
Should be called rememberPreview…
|
|
||
| /** Worker tests that use the [Worker.test] function. Core tests are in the core module. */ | ||
| class WorkerTest { | ||
| internal class WorkerTest { |
There was a problem hiding this comment.
any reason this became internal? given strict mode this would have been package private before?
There was a problem hiding this comment.
Because we use explicit API mode, the IDE floods tests with warnings about the need to specificy visibility. Setting the class to internal squelches that noise and makes warnings useful again.
I had originally thought about creating a new
ScreenComposableFactory : ViewRegistry.Entry, but that did nothing but introduce complexity. The existing system has the elegant property that everyViewFactorycan produce either a@Composableor aViewas needed. That's a crucial trait, and it's most easily maintained by teaching the same trick toScreenViewFactory.Where possible, existing tests are copied into new files with a
Legacyprefix and otherwise unchanged, so that diffs of the original tests will be reviewable.