-
Notifications
You must be signed in to change notification settings - Fork 67
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
Fixes Dropdown wrong default color #40
Conversation
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.
Thank you so much!
Can you regenerate test screenshots using and add them to this PR? ./gradlew cascade-compose:connectedDebugAndroidTest -Pdropshots.record |
I generated them using a Pixel 7 API 31. |
Made some small changes, I saw that it was setting the shadowElevation to the tonalElevation. And made a argument for tonalElevation |
There was a test with this remark
This was actually changing the tonalElevation and now it actually changes the shadowElevation Not sure if this is still needed? |
I'm sorry this took me some time. |
@@ -118,6 +119,7 @@ fun CascadeDropdownMenu( | |||
offset: DpOffset = DpOffset.Zero, | |||
fixedWidth: Dp = 196.dp, | |||
shadowElevation: Dp = 3.dp, | |||
tonalElevation: Dp = 3.dp, |
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.
Do you need your popups to have a different shadow and tonal elevations?
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 don't, but others might and this matches the Compose API as it gives both options
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.
Can you point me to that? I don't see any tonalElevation
parameter on DropdownMenu()
:
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.
According to the m3 specs, dropdown has a 3dp tonal elevation, this was missing and caused the discrepancy between DropdownMenu and CascadeDropdownMenu. See https://m3.material.io/styles/elevation/tokens#df721a00-888e-4c5e-bfe1-5d905f167aaa
After:
8Z8ul9eEY3.mp4
Before:
studio64_ydSz9zlk9O.mp4
Fixes #39