-
Notifications
You must be signed in to change notification settings - Fork 70
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
Conversation
kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/LayoutRunner.kt
Show resolved
Hide resolved
8a425af
to
b8ca7f9
Compare
.../samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloLayoutRunner.kt
Outdated
Show resolved
Hide resolved
1af7bc1
to
4a3befd
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.
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.
.../samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloLayoutRunner.kt
Outdated
Show resolved
Hide resolved
@@ -28,6 +28,8 @@ android { | |||
|
|||
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" | |||
} | |||
|
|||
buildFeatures.viewBinding = true |
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.
Shouldn't the need for this gradle config be called out in the kdoc?
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.
It definitely should
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.
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.
kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/AndroidXViewBinding.kt
Outdated
Show resolved
Hide resolved
kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/AndroidXViewBinding.kt
Outdated
Show resolved
Hide resolved
kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/LayoutRunner.kt
Outdated
Show resolved
Hide resolved
Looks like a legit UI test failure. |
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.
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.
|
c15e266
to
d462b53
Compare
6034291
to
d6aff34
Compare
@rjrjr I've already found a few. |
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. |
86ffdb3
to
47d8764
Compare
47d8764
to
427c2f8
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.
Wow, I'm thrilled with this.
Initially proposed by @0legg.
Closes #984.