Skip to content

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

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

rjrjr
Copy link
Contributor

@rjrjr rjrjr commented Jan 11, 2022

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.

@rjrjr rjrjr requested review from zach-klippenstein and a team as code owners January 11, 2022 17:59
@rjrjr rjrjr force-pushed the ray/ScreenViewFactory-extensions branch from 454f29d to e9c73c3 Compare January 11, 2022 22:36
@rjrjr rjrjr removed the request for review from steve-the-edwards January 12, 2022 16:31
Copy link
Contributor

@steve-the-edwards steve-the-edwards left a 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].
Copy link
Contributor

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?

Copy link
Contributor Author

@rjrjr rjrjr Jan 12, 2022

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.

Copy link
Contributor Author

@rjrjr rjrjr Jan 12, 2022

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.
@rjrjr rjrjr force-pushed the ray/ScreenViewFactory-extensions branch from e9c73c3 to 9c91a20 Compare January 12, 2022 17:49
@rjrjr
Copy link
Contributor Author

rjrjr commented Jan 12, 2022

To verify during Gradle-induced JCenter outage, running ./gradlew clean build connectedCheck locally

@rjrjr rjrjr merged commit b843cde into ray/ui-update Jan 12, 2022
@rjrjr rjrjr deleted the ray/ScreenViewFactory-extensions branch January 12, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants