-
Notifications
You must be signed in to change notification settings - Fork 40
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
Migration to Jetpack Compose. #179
Conversation
See https://developer.android.com/codelabs/jetpack-compose-migration#0 for background information. Step 1: very simple migration as a first step. bottom-up approach where we simply migrate the Home screen view subtree that is shown when no devices are currently commissioned to the ecosystem. Also, some gradle upgrades: - compileSdk: 33 -> 34 - Gradle plugin: 8.1.0 -> 8.1.1 - Kotlin plugin: 1.9.0 -> 1.9.20 - material: 1.9.0 -> 1.10.0
3p-ecosystem/src/main/java/com/google/homesampleapp/screens/home/HomeFragment.kt
Outdated
Show resolved
Hide resolved
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.
Just the minor change about the ViewModel being directly passed to the composable.
LGTM! Could you add me as a reviewer on next PRs so I can check them from a Compose perspective? Happy to answer any questions when they arise :) |
3p-ecosystem/src/main/java/com/google/homesampleapp/screens/home/HomeFragment.kt
Show resolved
Hide resolved
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.
LGTM, but should be reviewed by a Compose person to make sure
- The plugins block should be using the version catalog. - specify 'composeBom' version in version catalog - all deps should be in version catalog and referenced via libs.xxxxx... - Should be using the version catalog for plugins - The HomeScreen composable should take only lambdas and state if possible.
- Should update the versionCode and versionName - Delete the compileOptions and kotlinOptions blocks
Thanks Jolanda, will do. As mentioned via chat, it's not clear to me what's missing, but I cannot see the Composable previews in Android Studio. Note that I do have the following in my build.gradle.kts:
I'm using Giraffe | 2022.3.1 Patch 1. Update: |
@tunjid I looked at the code you referred to, and the ViewModel does get passed to InterestsRoute.
Not sure how I can get rid of it since in the composable we extract uiState from it. |
@pierredelisle in the interests snippet, there's a parent Composable; the The Your latest changes seem to already follow this approach. I made a few changes:
|
Build changes look good. |
- Pass DevicesUiModel to HomeScreen (instead of just a noDevices boolean)
Got it. Thanks for the clarifications! Made the changes. |
See https://developer.android.com/codelabs/jetpack-compose-migration#0 for background information.
Step 1: very simple migration as a first step.
bottom-up approach where we simply migrate the Home screen view subtree that is shown when no devices are currently commissioned to the ecosystem.
Also, some gradle upgrades: