-
Notifications
You must be signed in to change notification settings - Fork 24
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
Upcoming instructions android #326
Conversation
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/InstructionsView.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.
Looks good. Proposed a small compose improvement for iOS parity.
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/InstructionsView.kt
Outdated
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/InstructionsView.kt
Outdated
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/InstructionsView.kt
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/InstructionsView.kt
Outdated
Show resolved
Hide resolved
@@ -33,5 +33,5 @@ object DefaultInstructionRowTheme : InstructionRowTheme { | |||
@Composable get() = MaterialTheme.colorScheme.onSurface | |||
|
|||
override val backgroundColor: Color | |||
@Composable get() = MaterialTheme.colorScheme.surface | |||
@Composable get() = MaterialTheme.colorScheme.surfaceContainerLow |
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.
FWIW this came from the Material 3 spec
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.
My only comment on this is:
When material theme is used, the more complex the value is, the less likely someone has it in their theme. This isn't so much a problem. More just a warning when someone asks why their instructions row isn't changing color. It'll be because people rarely set those very nuanced values.
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.
Looking good!
Closes #248.
trim.29B08B24-CD4F-4104-9A7F-ED50EAB6DD2C.MOV