-
Notifications
You must be signed in to change notification settings - Fork 95
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
Adjust media layouts #1938
Adjust media layouts #1938
Conversation
7aeadbb
to
cc2ab3b
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.
After addressing comments.
@@ -37,8 +39,9 @@ public fun UampSettingsButtons( | |||
enabled: Boolean = true, | |||
) { | |||
Row( | |||
modifier = modifier, | |||
modifier = modifier.fillMaxWidth(0.8124f), |
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.
Should this be pulled up? Should these components care, or applied by the container above and passed in as modifier, or some box with extra constraints?
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.
This is still an open question - currently, one of the ideas is that if there's a single button, the whole lower area should be that one button, meaning that this can't be pulled up. However, I decided to make the current UAMP layout use the same recommended padding (9.38% on each side) so that we can prototype options for two and three button layouts this way.
media/ui/src/main/java/com/google/android/horologist/media/ui/components/ControlButtonLayout.kt
Outdated
Show resolved
Hide resolved
media/ui/src/main/java/com/google/android/horologist/media/ui/components/ControlButtonLayout.kt
Outdated
Show resolved
Hide resolved
media/ui/src/main/java/com/google/android/horologist/media/ui/components/MediaControlButtons.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.
Let's discuss in chat, but would be good to double check this is deliberate, given the gaps are there to avoid touching with RSB.
...s/com.google.android.horologist.media.ui_MediaPlayerA11yScreenshotTest_mediaPlayerSquare.png
Outdated
Show resolved
Hide resolved
...com.google.android.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerLargeRound.png
Outdated
Show resolved
Hide resolved
...edia.ui_PodcastPlayerScreenTest_mediaPlayerScreen[1]_known(seconds=10)_known(seconds=10).png
Outdated
Show resolved
Hide resolved
...m.google.android.horologist.screensizes_MediaPlayerTest_screenshot[0]_mobvoiticwatchpro5.png
Outdated
Show resolved
Hide resolved
cc @kul3r4 |
WHAT
Changes
media-ui
components to use the new recommended redlines (percent-based). This is the first part of a larger set of changes that will follow.WHY
Makes the UI cleaner on larger devices.
HOW
By using
ConstrantLayout
forPlayerScreen
.Checklist 📋