Adopt new context menu API #2214
Conversation
compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/uikit/LocalUIView.kt
Show resolved
Hide resolved
# Conflicts: # compose/foundation/foundation/src/desktopMain/kotlin/androidx/compose/foundation/text/input/internal/selection/TextFieldSelectionState.desktop.kt # compose/foundation/foundation/src/macosMain/kotlin/androidx/compose/foundation/text/input/internal/selection/TextFieldSelectionState.macos.kt # compose/foundation/foundation/src/uikitMain/kotlin/androidx/compose/foundation/text/input/internal/selection/TextFieldSelectionState.uikit.kt
compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/uikit/UIKitCompositionLocals.kt
Show resolved
Hide resolved
# Conflicts: # compose/foundation/foundation/src/uikitMain/kotlin/androidx/compose/foundation/text/input/internal/selection/TextFieldSelectionState.uikit.kt
| // menu items, which causes the context menu callout to blink. | ||
| // Adding a small delay resolves this issue. | ||
| ProvideDefaultPlatformTextContextMenuProviders( | ||
| menuDelay = 100.milliseconds, |
There was a problem hiding this comment.
Why 100? Does it work with 50? Can we use explicit trigger instead of delay?
There was a problem hiding this comment.
Yes, we can, but that we'll see glitches in context menu. Experimentally, 50 mills is not enough for some reasons.
| } | ||
| } | ||
|
|
||
| @OptIn(ExperimentalComposeUiApi::class) |
There was a problem hiding this comment.
Add TODO to remove cross-module experimental API usage
There was a problem hiding this comment.
I want to make the LocalUIView me a part of public API to ease platform integration for the developers.
Here will be an issue for that: https://youtrack.jetbrains.com/issue/CMP-8546/Remove-Experimental-flag-from-LocalUIView
| ) { | ||
| val toolbarProvider = LocalTextContextMenuToolbarProvider.current | ||
| val dropdownProvider = LocalTextContextMenuDropdownProvider.current | ||
| if (toolbarProvider == null || dropdownProvider == null) { |
There was a problem hiding this comment.
You need to attach the modifier and include the content even if both existing providers are non-null.
There was a problem hiding this comment.
Tnx, will be fixed in the next MR
|
|
||
| @OptIn(ExperimentalComposeUiApi::class) | ||
| @Composable | ||
| private fun ProvideDefaultPlatformTextContextMenuProviders( |
There was a problem hiding this comment.
We're creating a bit of a mess here.
There's already an expect ProvideDefaultPlatformTextContextMenuProviders function without the menuDelay argument, but we're not calling it here. In fact, it's neither actualized (except for an empty implementation) nor used in the uikit source set. So CommonContextMenuArea can't be used in this source-set.
@terrakok This looks messy. How should we clean it up?
There was a problem hiding this comment.
I'll rename those in uikit source set.
| LocalTextContextMenuToolbarProvider provides (toolbarProvider ?: provider), | ||
| LocalTextContextMenuDropdownProvider provides (dropdownProvider ?: provider), |
There was a problem hiding this comment.
You can use providesDefault here instead.
There was a problem hiding this comment.
Tnx! Will be fixed in the next MP
4 participants
Support ContextMenuArea for uiKit source set with default set of toolbar items.
Add
LocalUIViewto public (experimental for now) ui API to properly locate toolbar host view.Increase the delay before the first toolbar shows the action to mitigate glitches connected with delayed menu item provision.
Fixes https://youtrack.jetbrains.com/issue/CMP-8431/iOS-Adopt-new-context-menu-API
Fixes https://youtrack.jetbrains.com/issue/CMP-7906/iOS-Trackpad.-BTF2.-Sontext-menu-doesnt-appear
Release Notes
Features - iOS