From f2df70124b9602d2d11e73405f447d457b532b62 Mon Sep 17 00:00:00 2001 From: Abdelilah El Aissaoui Date: Mon, 27 May 2024 02:47:35 +0200 Subject: [PATCH] Fixed Context Menu inconsistent behavior in diff screen Also fixed empty context menu being shown when the list of items is empty (it required the user to click once to close this invisible popup and make other actions work properly) --- .../gitnuro/ui/context_menu/ContextMenu.kt | 39 +++++++----- .../ui/context_menu/CustomContextMenu.kt | 31 +++++++++- .../com/jetpackduba/gitnuro/ui/diff/Diff.kt | 59 +++++-------------- 3 files changed, 68 insertions(+), 61 deletions(-) diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/ContextMenu.kt b/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/ContextMenu.kt index fd10ef1..40c5020 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/ContextMenu.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/ContextMenu.kt @@ -2,7 +2,6 @@ package com.jetpackduba.gitnuro.ui.context_menu import androidx.compose.foundation.* import androidx.compose.foundation.layout.* -import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.text.TextContextMenu import androidx.compose.foundation.text.selection.DisableSelection import androidx.compose.material.Icon @@ -24,7 +23,9 @@ import androidx.compose.ui.graphics.painter.Painter import androidx.compose.ui.input.InputMode import androidx.compose.ui.input.InputModeManager import androidx.compose.ui.input.key.* +import androidx.compose.ui.input.pointer.PointerIcon import androidx.compose.ui.input.pointer.isSecondary +import androidx.compose.ui.input.pointer.pointerHoverIcon import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.platform.LocalFocusManager @@ -51,8 +52,8 @@ private var lastCheck: Long = 0 private const val MIN_TIME_BETWEEN_POPUPS_IN_MS = 20 @Composable -fun ContextMenu(items: () -> List, function: @Composable () -> Unit) { - Box(modifier = Modifier.contextMenu(items), propagateMinConstraints = true) { +fun ContextMenu(enabled: Boolean = true,items: () -> List, function: @Composable () -> Unit) { + Box(modifier = Modifier.contextMenu(enabled, items), propagateMinConstraints = true) { function() } } @@ -64,21 +65,23 @@ fun DropdownMenu(items: () -> List, function: @Composable () } } + @OptIn(ExperimentalComposeUiApi::class) @Composable -private fun Modifier.contextMenu(items: () -> List): Modifier { - val (lastMouseEventState, setLastMouseEventState) = remember { mutableStateOf(null) } +private fun Modifier.contextMenu(enabled: Boolean, items: () -> List): Modifier { + val (contentMenuData, setContentMenuData) = remember { mutableStateOf(null) } - val modifier = this.pointerInput(Unit) { + val modifier = this.pointerInput(enabled) { awaitPointerEventScope { while (true) { val lastMouseEvent = awaitFirstDownEvent() val mouseEvent = lastMouseEvent.awtEventOrNull - if (mouseEvent != null) { - + if (mouseEvent != null && enabled) { if (lastMouseEvent.button.isSecondary) { - lastMouseEvent.changes.forEach { it.consume() } + lastMouseEvent.changes.forEach { + it.consume() + } val currentCheck = System.currentTimeMillis() if (lastCheck != 0L && currentCheck - lastCheck < MIN_TIME_BETWEEN_POPUPS_IN_MS) { @@ -86,7 +89,7 @@ private fun Modifier.contextMenu(items: () -> List): Modifie } else { lastCheck = currentCheck - setLastMouseEventState(mouseEvent) + setContentMenuData(ContextMenuData(items(), mouseEvent)) } } } @@ -94,13 +97,13 @@ private fun Modifier.contextMenu(items: () -> List): Modifie } } - if (lastMouseEventState != null) { + if (contentMenuData != null && contentMenuData.items.isNotEmpty()) { DisableSelection { showPopup( - lastMouseEventState.x, - lastMouseEventState.y, - items(), - onDismissRequest = { setLastMouseEventState(null) } + contentMenuData.mouseEvent.x, + contentMenuData.mouseEvent.y, + contentMenuData.items, + onDismissRequest = { setContentMenuData(null) } ) } } @@ -108,6 +111,11 @@ private fun Modifier.contextMenu(items: () -> List): Modifie return modifier } +class ContextMenuData( + val items: List, + val mouseEvent: MouseEvent, +) + @Composable private fun Modifier.dropdownMenu(items: () -> List): Modifier { val (isClicked, setIsClicked) = remember { mutableStateOf(false) } @@ -237,6 +245,7 @@ fun TextEntry(contextTextEntry: ContextMenuElement.ContextTextEntry, onDismissRe onDismissRequest() contextTextEntry.onClick() } + .pointerHoverIcon(PointerIcon.Default) .padding(horizontal = 16.dp, vertical = 4.dp), verticalAlignment = Alignment.CenterVertically, ) { diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/CustomContextMenu.kt b/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/CustomContextMenu.kt index 6011827..465d81a 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/CustomContextMenu.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/CustomContextMenu.kt @@ -6,8 +6,15 @@ import androidx.compose.foundation.text.TextContextMenu import androidx.compose.runtime.Composable import androidx.compose.ui.text.AnnotatedString +/** + * This TextContextMenu will update the parent composable via @param onIsTextSelected when the text selection has changed. + * + * An example is the Diff screen, where lines can show different context menus depending on if text is selected or not. + * If nothing is selected, the default TextContentMenu should not be displayed and the parent composable can decide to + * show a context menu. + */ @OptIn(ExperimentalFoundationApi::class) -class CustomTextContextMenu(val onIsTextSelected: (AnnotatedString) -> Unit) : TextContextMenu { +class SelectionAwareTextContextMenu(val onIsTextSelected: (AnnotatedString) -> Unit) : TextContextMenu { @Composable override fun Area( textManager: TextContextMenu.TextManager, @@ -21,6 +28,26 @@ class CustomTextContextMenu(val onIsTextSelected: (AnnotatedString) -> Unit) : T println("Selected text check failed " + ex.message) } - content() + val emptyTextManager = object : TextContextMenu.TextManager { + override val copy: (() -> Unit)? + get() = null + override val cut: (() -> Unit)? + get() = null + override val paste: (() -> Unit)? + get() = null + override val selectAll: (() -> Unit)? + get() = null + override val selectedText: AnnotatedString + get() = AnnotatedString("") + + } + + val textManagerToUse = if (textManager.selectedText.isNotEmpty()) { + textManager + } else { + emptyTextManager + } + + AppPopupMenu().Area(textManagerToUse, state, content) } } \ No newline at end of file diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/ui/diff/Diff.kt b/src/main/kotlin/com/jetpackduba/gitnuro/ui/diff/Diff.kt index 4401cc7..12aa361 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/ui/diff/Diff.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/ui/diff/Diff.kt @@ -28,10 +28,6 @@ import androidx.compose.ui.input.key.onPreviewKeyEvent import androidx.compose.ui.input.key.type import androidx.compose.ui.input.pointer.PointerEventType import androidx.compose.ui.input.pointer.onPointerEvent -import androidx.compose.ui.platform.ClipboardManager -import androidx.compose.ui.platform.LocalClipboardManager -import androidx.compose.ui.platform.LocalLocalization -import androidx.compose.ui.platform.PlatformLocalization import androidx.compose.ui.res.loadImageBitmap import androidx.compose.ui.res.painterResource import androidx.compose.ui.text.AnnotatedString @@ -60,7 +56,7 @@ import com.jetpackduba.gitnuro.ui.components.SecondaryButton import com.jetpackduba.gitnuro.ui.components.tooltip.DelayedTooltip import com.jetpackduba.gitnuro.ui.context_menu.ContextMenu import com.jetpackduba.gitnuro.ui.context_menu.ContextMenuElement -import com.jetpackduba.gitnuro.ui.context_menu.CustomTextContextMenu +import com.jetpackduba.gitnuro.ui.context_menu.SelectionAwareTextContextMenu import com.jetpackduba.gitnuro.viewmodels.DiffViewModel import com.jetpackduba.gitnuro.viewmodels.TextDiffType import com.jetpackduba.gitnuro.viewmodels.ViewDiffResult @@ -442,11 +438,9 @@ fun HunkUnifiedTextDiff( ) { val hunks = diffResult.hunks var selectedText by remember { mutableStateOf(AnnotatedString("")) } - val localClipboardManager = LocalClipboardManager.current - val localization = LocalLocalization.current CompositionLocalProvider( - LocalTextContextMenu provides CustomTextContextMenu { + LocalTextContextMenu provides SelectionAwareTextContextMenu { selectedText = it } ) { @@ -477,8 +471,6 @@ fun HunkUnifiedTextDiff( items(hunk.lines) { line -> DiffContextMenu( selectedText = selectedText, - localization = localization, - localClipboardManager = localClipboardManager, diffEntryType = diffEntryType, onDiscardLine = { onDiscardLine(diffResult.diffEntry, hunk, line) }, line = line, @@ -525,7 +517,7 @@ fun HunkSplitTextDiff( var selectedText by remember { mutableStateOf(AnnotatedString("")) } CompositionLocalProvider( - LocalTextContextMenu provides CustomTextContextMenu { + LocalTextContextMenu provides SelectionAwareTextContextMenu { selectedText = it } ) { @@ -663,10 +655,6 @@ fun SplitDiffLineSide( var pressedAndMoved by remember(line) { mutableStateOf(Pair(false, false)) } var movesCount by remember(line) { mutableStateOf(0) } - val localClipboardManager = LocalClipboardManager.current - val localization = LocalLocalization.current - - Box( modifier = modifier .onPointerEvent(PointerEventType.Press) { @@ -703,8 +691,6 @@ fun SplitDiffLineSide( ) { DiffContextMenu( selectedText, - localization, - localClipboardManager, line, diffEntryType, onDiscardLine = { onDiscardLine(line) }, @@ -725,45 +711,30 @@ fun SplitDiffLineSide( @Composable fun DiffContextMenu( selectedText: AnnotatedString, - localization: PlatformLocalization, - localClipboardManager: ClipboardManager, line: Line, diffEntryType: DiffEntryType, onDiscardLine: () -> Unit, content: @Composable () -> Unit, ) { ContextMenu( + enabled = selectedText.isEmpty(), items = { - val isTextSelected = selectedText.isNotEmpty() - - if (isTextSelected) { + if ( + line.lineType != LineType.CONTEXT && + diffEntryType is DiffEntryType.UnstagedDiff && + diffEntryType.statusType == StatusType.MODIFIED + ) { listOf( ContextMenuElement.ContextTextEntry( - label = localization.copy, - icon = { painterResource(AppIcons.COPY) }, + label = "Discard line", + icon = { painterResource(AppIcons.UNDO) }, onClick = { - localClipboardManager.setText(selectedText) + onDiscardLine() } ) ) - } else { - if ( - line.lineType != LineType.CONTEXT && - diffEntryType is DiffEntryType.UnstagedDiff && - diffEntryType.statusType == StatusType.MODIFIED - ) { - listOf( - ContextMenuElement.ContextTextEntry( - label = "Discard line", - icon = { painterResource(AppIcons.UNDO) }, - onClick = { - onDiscardLine() - } - ) - ) - } else - emptyList() - } + } else + emptyList() }, ) { content() @@ -869,7 +840,7 @@ private fun DiffHeader( .weight(1f, true) ) { SelectionContainer { - Row() { + Row { if (dirPath.isNotEmpty()) { Text( text = dirPath.removeSuffix("/"),