-
Notifications
You must be signed in to change notification settings - Fork 108
Introduces ScreenViewFactoryFinder #625
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.
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
Conversation
454f29d
to
e9c73c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but confused on the kdoc description on one point.
/** | ||
* [ViewEnvironment] service object used by [Screen.buildView] to find the right | ||
* [ScreenViewFactory]. The default implementation is why [AndroidScreen], etc. work. | ||
* Can be customized like any other [ViewEnvironment] entry, e.g. via [EnvironmentScreen]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if I understand this last sentence about [EnvironmentScreen]
being the example of a customization of the [ViewEnvironment]
entry. You mean using an [EnvironmentScreen]
to extend the [ViewEnvironment]
by adding a new map? This seems slightly different than 'customizing like any other [ViewEnvironment] entry'. E.g. I can change the map values wherever I have access to the viewEnvironment
, using [EnvironmentScreen]
seems like something else. Am I misunderstanding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just saying that EnvironmentScreen
at the root of an app is the easiest way to do it. It's confusing, I'll just delete that phrase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* [ViewEnvironment] service object used by [Screen.buildView] to find the right
* [ScreenViewFactory]. The default implementation makes [AndroidScreen] work
* and provides default bindings for [NamedScreen], [EnvironmentScreen], [BackStackScreen],
* etc.
*
* Here is how this hook could be used to provide a custom view to handle [BackStackScreen]:
*
* object MyViewFactory : ScreenViewFactory<BackStackScreen<*>>
* by ManualScreenViewFactory(
* type = BackStackScreen::class,
* viewConstructor = { initialRendering, initialEnv, context, _ ->
* MyBackStackContainer(context)
* .apply {
* layoutParams = (LayoutParams(MATCH_PARENT, MATCH_PARENT))
* bindShowRendering(initialRendering, initialEnv, ::update)
* }
* }
* )
*
* object MyFinder : ScreenViewFactoryFinder {
* @Suppress("UNCHECKED_CAST")
* if (rendering is BackStackScreen<*>)
* return MyViewFactory as ScreenViewFactory<ScreenT>
* return super.getViewFactoryForRendering(environment, rendering)
* }
*
* class MyViewModel(savedState: SavedStateHandle) : ViewModel() {
* val renderings: StateFlow<MyRootRendering> by lazy {
* val customized = ViewEnvironment() + (ScreenViewFactoryFinder to MyFinder)
* renderWorkflowIn(
* workflow = MyRootWorkflow.withEnvironment(customized),
* scope = viewModelScope,
* savedStateHandle = savedState
* )
* }
* }
*/
Fixes #594, which was about how impractical it is to wrap the `ViewRegistry` interface, by introducing a higher level interface that's easy to wrap. The problem is that the fundamental method is `getFactoryFor(KClass)`, but lots of crucial behavior (e.g. `AndroidViewRendering` / `AndroidScreenRendering`) is in the `getFactoryForRendering` extension method. But if we change the fundamental method to be instance based instead of type based, we bring back a lot of potential complexity to `ViewRegistry` that the original type-based choice very intentionally restricted. To have our cake and eat it too, we move the extension method to a new `ViewEnvironment` service interface, `ScreenViewFactoryFinder`. Ta da, totally customizable, with the defaults totally built in.
e9c73c3
to
9c91a20
Compare
To verify during Gradle-induced JCenter outage, running |
Fixes #594, which was about how impractical it is to wrap the
ViewRegistry
interface, by introducing a higher level interface that's easy to wrap.
The problem is that the fundamental method is
getFactoryFor(KClass)
, butlots of crucial behavior (e.g.
AndroidViewRendering
/AndroidScreenRendering
) is in thegetFactoryForRendering
extensionmethod. But if we change the fundamental method to be instance based instead
of type based, we bring back a lot of potential complexity to
ViewRegistry
that the original type-based choice very intentionally restricted.
To have our cake and eat it too, we move the extension method to a new
ViewEnvironment
service interface,ScreenViewFactoryFinder
. Ta da,totally customizable, with the defaults totally built in.