-
Notifications
You must be signed in to change notification settings - Fork 84
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
Expect material3.ExposedDropdownMenuBox
in common
#787
Conversation
b2bffb2
to
f2b7055
Compare
transformOriginState.value = calculateTransformOrigin(anchorBounds, menuBounds) | ||
} | ||
@Composable | ||
actual fun ExposedDropdownMenuBoxScope.ExposedDropdownMenu( |
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 will break binary backwards-compatibility on Android; not sure how important it is, though. @igordmn can say better.
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.
interface and extensions functions also differ on source level, so it is undesirable to have such difference.
It seems, we can just copy ExposedDropdownMenuBoxScope
to commonMain, and make only ExposedDropdownMenuPopup
expect/actual, instead of the full interface?
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.
Extracted expect/actual into internal function
} | ||
|
||
@OptIn(ExperimentalComposeUiApi::class) | ||
private fun updateHeight( |
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 can be simplified to just return the new height, but could make it harder to merge changes from upstream. Not sure what takes precedence.
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 would simplify it. Most probably we won't be updating updateHeight from upstream, as it depends on other pieces.
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.
Simplified
6404eeb
to
23c9367
Compare
...material3/material3/src/androidMain/kotlin/androidx/compose/material3/ExposedDropdownMenu.kt
Outdated
Show resolved
Hide resolved
...rial3/material3/src/skikoMain/kotlin/androidx/compose/material3/ExposedDropdownMenu.skiko.kt
Outdated
Show resolved
Hide resolved
...rial3/material3/src/skikoMain/kotlin/androidx/compose/material3/ExposedDropdownMenu.skiko.kt
Outdated
Show resolved
Hide resolved
material3.ExposedDropdownMenuBox
in common
* Expect ExposedDropdownMenuBox in common * Add ExposedDropdownMenu3Sample * Update to windowInfo.containerSize * Move ExposedDropdownMenuDefaults to common * Simplify updateHeight * Fix import
…enuBox` in common (#787) * Expect ExposedDropdownMenuBox in common * Add ExposedDropdownMenu3Sample * Update to windowInfo.containerSize * Move ExposedDropdownMenuDefaults to common * Simplify updateHeight * Fix import
* Expect ExposedDropdownMenuBox in common * Add ExposedDropdownMenu3Sample * Update to windowInfo.containerSize * Move ExposedDropdownMenuDefaults to common * Simplify updateHeight * Fix import
Proposed Changes
ExposedDropdownMenuBox
in common and implement for skikoAPI Changes
Testing
Test: Try to use
ExposedDropdownMenuBox
from common or look at mpp demoIssues Fixed
Fixes JetBrains/compose-multiplatform#2037