Skip to content

Commit

Permalink
Fixed Context Menu inconsistent behavior in diff screen
Browse files Browse the repository at this point in the history 
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)
  • Loading branch information
@JetpackDuba
JetpackDuba committed May 27, 2024
1 parent 62c9f11 commit f2df701
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 61 deletions.
39 changes: 24 additions & 15 deletions src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/ContextMenu.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -51,8 +52,8 @@ private var lastCheck: Long = 0
private const val MIN_TIME_BETWEEN_POPUPS_IN_MS = 20

@Composable
fun ContextMenu(items: () -> List<ContextMenuElement>, function: @Composable () -> Unit) {
Box(modifier = Modifier.contextMenu(items), propagateMinConstraints = true) {
fun ContextMenu(enabled: Boolean = true,items: () -> List<ContextMenuElement>, function: @Composable () -> Unit) {
Box(modifier = Modifier.contextMenu(enabled, items), propagateMinConstraints = true) {
function()
}
}
Expand All @@ -64,50 +65,57 @@ fun DropdownMenu(items: () -> List<ContextMenuElement>, function: @Composable ()
}
}


@OptIn(ExperimentalComposeUiApi::class)
@Composable
private fun Modifier.contextMenu(items: () -> List<ContextMenuElement>): Modifier {
val (lastMouseEventState, setLastMouseEventState) = remember { mutableStateOf<MouseEvent?>(null) }
private fun Modifier.contextMenu(enabled: Boolean, items: () -> List<ContextMenuElement>): Modifier {
val (contentMenuData, setContentMenuData) = remember { mutableStateOf<ContextMenuData?>(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) {
println("Popup ignored!")
} else {
lastCheck = currentCheck

setLastMouseEventState(mouseEvent)
setContentMenuData(ContextMenuData(items(), mouseEvent))
}
}
}
}
}
}

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) }
)
}
}

return modifier
}

class ContextMenuData(
val items: List<ContextMenuElement>,
val mouseEvent: MouseEvent,
)

@Composable
private fun Modifier.dropdownMenu(items: () -> List<ContextMenuElement>): Modifier {
val (isClicked, setIsClicked) = remember { mutableStateOf(false) }
Expand Down Expand Up @@ -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,
) {
Expand Down
31 changes: 29 additions & 2 deletions src/main/kotlin/com/jetpackduba/gitnuro/ui/context_menu/CustomContextMenu.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
}
59 changes: 15 additions & 44 deletions src/main/kotlin/com/jetpackduba/gitnuro/ui/diff/Diff.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -525,7 +517,7 @@ fun HunkSplitTextDiff(
var selectedText by remember { mutableStateOf(AnnotatedString("")) }

CompositionLocalProvider(
LocalTextContextMenu provides CustomTextContextMenu {
LocalTextContextMenu provides SelectionAwareTextContextMenu {
selectedText = it
}
) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -703,8 +691,6 @@ fun SplitDiffLineSide(
) {
DiffContextMenu(
selectedText,
localization,
localClipboardManager,
line,
diffEntryType,
onDiscardLine = { onDiscardLine(line) },
Expand All @@ -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()
Expand Down Expand Up @@ -869,7 +840,7 @@ private fun DiffHeader(
.weight(1f, true)
) {
SelectionContainer {
Row() {
Row {
if (dirPath.isNotEmpty()) {
Text(
text = dirPath.removeSuffix("/"),
Expand Down

0 comments on commit f2df701

Please sign in to comment.