-
Notifications
You must be signed in to change notification settings - Fork 108
Disable, rather than remove items from the (old) desktop text context menus #2617
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
Conversation
d1cd25d to
e4fefcf
Compare
...on/foundation/src/desktopMain/kotlin/androidx/compose/foundation/text/ContextMenu.desktop.kt
Outdated
Show resolved
Hide resolved
| private fun SemanticsNodeInteraction.assertMenuItemDoesNotExistOrIsDisabled() { | ||
| // New context menus disabled items; old context menus don't show disabled items. | ||
| if (ComposeFoundationFlags.isNewContextMenuEnabled) { | ||
| assertIsNotEnabled() | ||
| } else { | ||
| assertDoesNotExist() | ||
| } | ||
| } | ||
|
|
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 think you'll still need this if you add tests for the "hide disabled items" menu, which seem to be missing
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.
contextMenuArea disables or removes copy action for SelectionContainer with empty selection tests this once, but since the default behavior is now to disable the menu item, there's no need for this function (and also the behavior no longer depends on isNewContextMenuEnabled, but on LocalTextContextMenu).
…nager.textManager`
...on/foundation/src/desktopMain/kotlin/androidx/compose/foundation/text/ContextMenu.desktop.kt
Outdated
Show resolved
Hide resolved
...foundation/src/desktopMain/kotlin/androidx/compose/foundation/ContextMenuProvider.desktop.kt
Outdated
Show resolved
Hide resolved
...foundation/src/desktopMain/kotlin/androidx/compose/foundation/ContextMenuProvider.desktop.kt
Outdated
Show resolved
Hide resolved
igordmn
left a comment
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.
Needs a second approval
|
terrakok
left a comment
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.
It seems OK for me, but I'm far from Desktop target API and specifics
TextContextMenucan now create disabled menu items viaContextMenuItemWithEnabledStateand they will be shown as such in the context menu.The default
TextContextMenuimplementation (TextContextMenu.Default) utilizes this to disable, rather than remove irrelevant menu items. For example, when there is no selection in a text field, the "Copy" menu item will be disabled rather than hidden.To get the previous behavior where the irrelevant item is hidden, use
TextContextMenu.HideDisabledMenuItemslike so:Note that this is all relevant only for the old text context menus (when
ComposeFoundationFlags.isNewContextMenuEnabledisfalse).Fixes https://youtrack.jetbrains.com/issue/CMP-9329
Fixes https://youtrack.jetbrains.com/issue/CMP-9342
Testing
Added unit tests and also tested manually
This should be tested by QA
Release Notes
Fixes - Desktop
SelectionContainer, the "Copy" menu item will be disabled.Features - Desktop
SelectionContainercan now be disabled.