Skip to content
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

Simplify ViewRegistry to act more like a simple map of rendering type to ViewFactory. #1148

Merged
merged 1 commit into from May 11, 2020

Conversation

zach-klippenstein
Copy link
Collaborator

ViewRegistry is now just responsible for returning a ViewFactory for a given
rendering type, as well as checking that views returned from the factory have been
correctly bound with bindShowRendering. buildView has been extracted into an
extension method on ViewRegistry.

This change simplifies the responsibility of each ViewRegistry implementation. It
also makes ViewRegistry more flexible for extension. For example, allows a
Compose-based ViewFactory to detect recursive Compose bindings and stay in Compose
instead of jumping back out into the legacy view layer every time.

Checklist

  • Unit Tests
  • UI Tests
  • Snapshot Tests (iOS only)
  • I have made corresponding changes to the documentation

@zach-klippenstein zach-klippenstein added enhancement New feature or request kotlin Affects the Kotlin library. labels May 10, 2020
@zach-klippenstein zach-klippenstein added this to the v1.0.0 milestone May 10, 2020
Copy link
Contributor

@rjrjr rjrjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -27,8 +27,8 @@ import kotlin.reflect.KClass
*
* Sets of bindings are gathered in [ViewRegistry] instances.
*/
interface ViewFactory<RenderingT : Any> {
val type: KClass<RenderingT>
interface ViewFactory<in RenderingT : Any> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that's a good sign.

@rjrjr rjrjr added this to Reviewer approved in Workflow via automation May 11, 2020
… to ViewFactory.

`ViewRegistry` is now just responsible for returning a `ViewFactory` for a given
rendering type, as well as checking that views returned from the factory have been
correctly bound with `bindShowRendering`. `buildView` has been extracted into an
extension method on `ViewRegistry`.

This change simplifies the responsibility of each `ViewRegistry` implementation. It
also makes `ViewRegistry` more flexible for extension. For example, allows a
Compose-based `ViewFactory` to detect recursive Compose bindings and stay in Compose
instead of jumping back out into the legacy view layer every time.
@zach-klippenstein
Copy link
Collaborator Author

UI test failures are installation network exceptions.

@zach-klippenstein zach-klippenstein merged commit 3530f3b into master May 11, 2020
Workflow automation moved this from Reviewer approved to Done May 11, 2020
@zach-klippenstein zach-klippenstein deleted the zachklipp/getviewfactory branch May 11, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kotlin Affects the Kotlin library.
Projects
Workflow
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants