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

Introduce a view binding function that works with AndroidX ViewBindings. #985

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

zach-klippenstein
Copy link
Collaborator

@zach-klippenstein zach-klippenstein commented Feb 26, 2020

Initially proposed by @0legg.

Closes #984.

val HelloBinding: ViewBinding<HelloWorkflow.Rendering> =
  bindViewBinding(HelloGoodbyeLayoutBinding::inflate) { rendering, containerHints ->
    helloMessage.text = rendering.message
    helloMessage.setOnClickListener { rendering.onClick }
  }

@zach-klippenstein zach-klippenstein changed the title WIP Create a LayoutRunner binding function that works with AndroidX ViewBindings. WIP Introduce a view binding function that works with AndroidX ViewBindings. Feb 27, 2020
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/androidx-viewbinding branch 3 times, most recently from 8a425af to b8ca7f9 Compare February 27, 2020 14:14
@zach-klippenstein zach-klippenstein changed the title WIP Introduce a view binding function that works with AndroidX ViewBindings. Introduce a view binding function that works with AndroidX ViewBindings. Feb 27, 2020
@zach-klippenstein zach-klippenstein marked this pull request as ready for review February 27, 2020 14:30
@zach-klippenstein zach-klippenstein added the enhancement New feature or request label Feb 27, 2020
@zach-klippenstein zach-klippenstein added this to the kotlin 0.24.0 milestone Feb 27, 2020
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/androidx-viewbinding branch 2 times, most recently from 1af7bc1 to 4a3befd Compare February 27, 2020 14:37
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.

I messed with the kdoc a bit, and suggested slightly different packaging for the anonymous binding method.

We really need to find a non-conflicting name for our own ViewBinding class, but let's address that after this lands.

@@ -28,6 +28,8 @@ android {

testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
}

buildFeatures.viewBinding = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the need for this gradle config be called out in the kdoc?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It definitely should

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be if we want to recommend ViewBinding as the best practice. I was thinking of this more as "if you want to use ViewBinding, we'll make it easy for you", and "if you want to use ViewBinding" implies that you know how to turn it on. But I would not be opposed to recommending ViewBinding as the default over raw views.

@zach-klippenstein
Copy link
Collaborator Author

Looks like a legit UI test failure.

Copy link
Collaborator

@0legg 0legg left a comment

Choose a reason for hiding this comment

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

So far it looks good to me. We definitely should rename Workflow's very own ViewBinding, because we've already had been confused multiple times internally, and who knows how tricky this terminology will be externally. I feel Android View Binding is ready for production use – it's actually slimmed down Data Binding, and Data Binding is stable for a long time.

@zach-klippenstein zach-klippenstein added the kotlin Affects the Kotlin library. label Feb 28, 2020
@zach-klippenstein
Copy link
Collaborator Author

ViewBinding rename is filed as #988.

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/androidx-viewbinding branch 5 times, most recently from c15e266 to d462b53 Compare March 6, 2020 19:18
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/androidx-viewbinding branch 2 times, most recently from 6034291 to d6aff34 Compare March 7, 2020 00:26
@zach-klippenstein
Copy link
Collaborator Author

Do any of the other current samples do the kind of pre-render setup that would justify LayoutRunner + Binding

@rjrjr I've already found a few.

@zach-klippenstein
Copy link
Collaborator Author

I'm going to only convert the HelloWorkflow sample in this PR, and then convert all the other samples in a follow-up so it doesn't block the 0.24.0 branch cut. See #1012.

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/androidx-viewbinding branch 2 times, most recently from 86ffdb3 to 47d8764 Compare March 7, 2020 02:46
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.

Wow, I'm thrilled with this.

@zach-klippenstein zach-klippenstein merged commit 3cd92e6 into master Mar 9, 2020
@zach-klippenstein zach-klippenstein deleted the zachklipp/androidx-viewbinding branch March 9, 2020 20:21
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
None yet
Development

Successfully merging this pull request may close these issues.

Explore AndroidX ViewBinding integration for LayoutRunner
3 participants