-
Notifications
You must be signed in to change notification settings - Fork 600
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
Upgrade Compose to 1.6.0-alpha06 #1715
Conversation
@@ -12,7 +12,7 @@ gradlePlugin = "8.1.0" | |||
lintMinCompose = "30.0.0" | |||
|
|||
ktlint = "0.45.2" | |||
kotlin = "1.9.0" |
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.
Not blocking - is bumping Kotlin and the compiler require from accompanist? Just wondering if the transitive Kotlin bump here is worth it (or if that bump would be transitive anyway from base Compose)
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.
We always just release with the latest compiler to make sure we stay up to date with fixes there. I haven't heard an issue yet with an incompatible change
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.
Not blocking - is bumping Kotlin and the compiler require from accompanist? Just wondering if the transitive Kotlin bump here is worth it (or if that bump would be transitive anyway from base Compose)
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.
@@ -68,7 +67,6 @@ class NavDrawerFoldAwareColumnSample : ComponentActivity() { | |||
@Composable | |||
fun NavDrawerExample(activity: Activity) { | |||
val icons = listOf( | |||
Icons.Default.List, |
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.
Nit: Was this removed? Or was this one of the AutoMirrored
icons?
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 was automirrored but the label looks weird in the sample because of splitting at the first .
. It seemed easier to just remove it rather than fix that
Fixes #1714