diff --git a/ComposeTextEditor/build.gradle.kts b/ComposeTextEditor/build.gradle.kts index 89bd04d..5076e51 100644 --- a/ComposeTextEditor/build.gradle.kts +++ b/ComposeTextEditor/build.gradle.kts @@ -65,6 +65,9 @@ kotlin { implementation(libs.mockk) implementation(libs.kotlinx.coroutines.test) implementation(libs.kotlinx.coroutines.test.jvm) + @OptIn(org.jetbrains.compose.ExperimentalComposeLibrary::class) + implementation(compose.uiTest) + implementation(compose.desktop.currentOs) } } } diff --git a/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/input/TextEditorKeyCommandHandler.kt b/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/input/TextEditorKeyCommandHandler.kt index 6fc816c..8417253 100644 --- a/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/input/TextEditorKeyCommandHandler.kt +++ b/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/input/TextEditorKeyCommandHandler.kt @@ -331,30 +331,7 @@ internal class TextEditorKeyCommandHandler { state: TextEditorState, initialPosition: CharLineOffset ) { - val currentSelection = state.selector.selection - when { - // No existing selection - start a new one - currentSelection == null -> { - state.selector.updateSelection(initialPosition, state.cursorPosition) - } - // Extend/modify existing selection - else -> { - // If we were at the start of the selection, keep the end fixed - when (initialPosition) { - currentSelection.start -> { - state.selector.updateSelection(state.cursorPosition, currentSelection.end) - } - // If we were at the end of the selection, keep the start fixed - currentSelection.end -> { - state.selector.updateSelection(currentSelection.start, state.cursorPosition) - } - // If cursor was outside selection, create new selection - else -> { - state.selector.updateSelection(initialPosition, state.cursorPosition) - } - } - } - } + state.selector.extendSelection(initialPosition, state.cursorPosition) } /** diff --git a/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/state/TextEditorSelectionManager.kt b/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/state/TextEditorSelectionManager.kt index 4bcd747..967038c 100644 --- a/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/state/TextEditorSelectionManager.kt +++ b/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/state/TextEditorSelectionManager.kt @@ -64,6 +64,28 @@ class TextEditorSelectionManager( updateSelectionRange(range) } + /** + * Extends the selection to [newPosition], holding the anchor fixed, and returns the fixed + * anchor. The anchor is the end of the existing selection opposite to [anchor]; with no + * selection, [anchor] itself becomes the anchor. Collapsing onto the anchor clears the + * selection. Shared by shift+arrow keys and shift+click so both extend identically. + */ + fun extendSelection(anchor: CharLineOffset, newPosition: CharLineOffset): CharLineOffset { + val currentSelection = _selection + val fixedAnchor = when { + currentSelection == null -> anchor + anchor == currentSelection.start -> currentSelection.end + anchor == currentSelection.end -> currentSelection.start + else -> anchor + } + if (fixedAnchor == newPosition) { + clearSelection() + } else { + updateSelection(fixedAnchor, newPosition) + } + return fixedAnchor + } + private fun makeRange(start: CharLineOffset, end: CharLineOffset): TextEditorRange { return if (isBeforeInDocument(start, end)) { TextEditorRange(start, end) diff --git a/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/textEditorPointerInputHandling.kt b/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/textEditorPointerInputHandling.kt index 3dcd271..2b0dd4d 100644 --- a/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/textEditorPointerInputHandling.kt +++ b/ComposeTextEditor/src/commonMain/kotlin/com/darkrockstudios/texteditor/textEditorPointerInputHandling.kt @@ -28,12 +28,16 @@ internal fun Modifier.textEditorPointerInputHandling( .handleDragInput(state, readOnly) .handleTextInteractions(state, onSpanClick, onContextMenuRequest, readOnly) .detectMouseClicksImperatively( - onClick = { offset: Offset -> - val position = state.getOffsetAtPosition(offset) - if (!readOnly) { - state.cursor.updatePosition(position) + onClick = { offset: Offset, isShiftPressed: Boolean -> + // On shift+click handleDragInput extends the selection; clearing it here + // (handlers run in parallel) would race away the extension. + if (!isShiftPressed) { + val position = state.getOffsetAtPosition(offset) + if (!readOnly) { + state.cursor.updatePosition(position) + } + state.selector.clearSelection() } - state.selector.clearSelection() }, onDoubleClick = { offset: Offset -> val position = state.getOffsetAtPosition(offset) @@ -60,6 +64,7 @@ private fun Modifier.handleDragInput(state: TextEditorState, readOnly: Boolean): !currentEvent.buttons.isSecondaryPressed val isMouseLike = down.type == PointerType.Mouse || hasPrimaryButton val isFingerTouch = down.type == PointerType.Touch && !hasPrimaryButton + val isShiftPressed = currentEvent.keyboardModifiers.isShiftPressed val initialPosition = down.position @@ -73,8 +78,19 @@ private fun Modifier.handleDragInput(state: TextEditorState, readOnly: Boolean): } else if (isMouseLike && hasPrimaryButton) { // Only start selection drag on primary (left) mouse button // Secondary (right) click should preserve existing selection for context menu - mouseSelectionAnchor = state.getOffsetAtPosition(initialPosition) - state.selector.startSelection(position = mouseSelectionAnchor, isTouch = false) + val clickedPosition = state.getOffsetAtPosition(initialPosition) + if (isShiftPressed) { + // This handler owns the shift+click extension; the other two pointer + // handlers must leave the selection alone while shift is held. + val anchor = state.cursorPosition + mouseSelectionAnchor = state.selector.extendSelection(anchor, clickedPosition) + if (!readOnly) { + state.cursor.updatePosition(clickedPosition) + } + } else { + mouseSelectionAnchor = clickedPosition + state.selector.startSelection(position = clickedPosition, isTouch = false) + } } val pointerId = down.id @@ -155,6 +171,7 @@ private fun handleSpanInteraction( clickType: SpanClickType, onSpanClick: RichSpanClickListener?, readOnly: Boolean, + isShiftPressed: Boolean = false, ): Boolean { if (findHandleAtPosition(offset, state) != null) { return true @@ -163,14 +180,16 @@ private fun handleSpanInteraction( val position = state.getOffsetAtPosition(offset) val clickedSpan = state.findSpanAtPosition(position) - if (clickType == SpanClickType.PRIMARY_CLICK || clickType == SpanClickType.TAP) { + // A shift+click extends the selection; handleDragInput owns that, so leave the + // cursor and selection untouched here rather than collapsing them. + if (!isShiftPressed && (clickType == SpanClickType.PRIMARY_CLICK || clickType == SpanClickType.TAP)) { if (!readOnly) { state.cursor.updatePosition(position) } state.selector.clearSelection() } - return if (clickedSpan != null && onSpanClick != null) { + return if (!isShiftPressed && clickedSpan != null && onSpanClick != null) { onSpanClick.invoke(clickedSpan, clickType, offset) } else { true @@ -226,6 +245,7 @@ private fun Modifier.handleTextInteractions( SpanClickType.PRIMARY_CLICK, onSpanClick, readOnly, + isShiftPressed = event.keyboardModifiers.isShiftPressed, ) didHandlePress = true } else if (hasSecondaryButton) { @@ -312,7 +332,7 @@ private fun Modifier.handleTextInteractions( @OptIn(ExperimentalTime::class) private fun Modifier.detectMouseClicksImperatively( - onClick: (Offset) -> Unit, + onClick: (Offset, Boolean) -> Unit, onDoubleClick: (Offset) -> Unit, onTripleClick: (Offset) -> Unit, ): Modifier { @@ -343,7 +363,7 @@ private fun Modifier.detectMouseClicksImperatively( val downTime = kotlin.time.Clock.System.now().toEpochMilliseconds() val downPosition = down.position - onClick(downPosition) + onClick(downPosition, currentEvent.keyboardModifiers.isShiftPressed) do { val event = awaitPointerEvent() diff --git a/ComposeTextEditor/src/desktopTest/kotlin/selection/ShiftClickExtendSelectionTest.kt b/ComposeTextEditor/src/desktopTest/kotlin/selection/ShiftClickExtendSelectionTest.kt new file mode 100644 index 0000000..f7d6254 --- /dev/null +++ b/ComposeTextEditor/src/desktopTest/kotlin/selection/ShiftClickExtendSelectionTest.kt @@ -0,0 +1,119 @@ +package selection + +import androidx.compose.ui.text.AnnotatedString +import com.darkrockstudios.texteditor.CharLineOffset +import com.darkrockstudios.texteditor.state.TextEditorState +import io.mockk.mockk +import kotlinx.coroutines.test.TestScope +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * Regression for issue #23 — shift+click should reliably extend the selection. + * + * Exercises the selection math the pointer handlers drive on a shift+click: a plain + * click positions the cursor and clears any selection; a shift+click extends from the + * cursor to the click while holding the original anchor fixed. + */ +class ShiftClickExtendSelectionTest { + + private lateinit var state: TextEditorState + + @BeforeTest + fun setup() { + state = TextEditorState( + scope = TestScope(), + measurer = mockk(relaxed = true), + initialText = AnnotatedString(""), + ) + state.setText("The quick brown fox jumps over the lazy dog") + } + + private fun offset(char: Int) = CharLineOffset(0, char) + + /** Mirrors a plain primary click: move cursor, drop any selection. */ + private fun click(char: Int) { + state.cursor.updatePosition(offset(char)) + state.selector.clearSelection() + } + + /** Mirrors a shift+primary click: extend from the cursor to the click, then move the cursor. */ + private fun shiftClick(char: Int) { + val anchor = state.cursorPosition + state.selector.extendSelection(anchor, offset(char)) + state.cursor.updatePosition(offset(char)) + } + + @Test + fun `shift+click with no selection selects from cursor to click`() { + click(4) + shiftClick(15) + + val selection = state.selector.selection + assertEquals(offset(4), selection?.start) + assertEquals(offset(15), selection?.end) + } + + @Test + fun `shift+click works the same selecting backward`() { + click(15) + shiftClick(4) + + val selection = state.selector.selection + assertEquals(offset(4), selection?.start) + assertEquals(offset(15), selection?.end) + } + + @Test + fun `a second shift+click keeps the original anchor fixed`() { + click(4) + shiftClick(9) + shiftClick(19) + + val selection = state.selector.selection + assertEquals(offset(4), selection?.start, "anchor from the first click must stay put") + assertEquals(offset(19), selection?.end) + } + + @Test + fun `shift+click can shrink the selection back toward the anchor`() { + click(4) + shiftClick(19) + shiftClick(9) + + val selection = state.selector.selection + assertEquals(offset(4), selection?.start) + assertEquals(offset(9), selection?.end) + } + + @Test + fun `shift+click crossing the anchor flips the selection around it`() { + click(10) + shiftClick(19) + shiftClick(4) + + val selection = state.selector.selection + assertEquals(offset(4), selection?.start) + assertEquals(offset(10), selection?.end, "anchor stays at the original click") + } + + @Test + fun `shift+click back onto the anchor collapses the selection`() { + click(4) + shiftClick(19) + shiftClick(4) + + assertNull(state.selector.selection) + } + + @Test + fun `a plain click after extending clears the selection`() { + click(4) + shiftClick(19) + click(9) + + assertNull(state.selector.selection) + } +} diff --git a/ComposeTextEditor/src/desktopTest/kotlin/selection/ShiftClickPointerInputTest.kt b/ComposeTextEditor/src/desktopTest/kotlin/selection/ShiftClickPointerInputTest.kt new file mode 100644 index 0000000..7e4c2c6 --- /dev/null +++ b/ComposeTextEditor/src/desktopTest/kotlin/selection/ShiftClickPointerInputTest.kt @@ -0,0 +1,102 @@ +package selection + +import androidx.compose.foundation.layout.size +import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.input.key.Key +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.click +import androidx.compose.ui.test.onRoot +import androidx.compose.ui.test.performKeyInput +import androidx.compose.ui.test.performMouseInput +import androidx.compose.ui.test.runComposeUiTest +import androidx.compose.ui.text.AnnotatedString +import androidx.compose.ui.unit.dp +import com.darkrockstudios.texteditor.BasicTextEditor +import com.darkrockstudios.texteditor.state.TextEditorState +import com.darkrockstudios.texteditor.state.rememberTextEditorState +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * Drives the real [Modifier.textEditorPointerInputHandling] through [BasicTextEditor] to guard + * the handler race in issue #23: three parallel pointerInput handlers must not clear the + * selection a shift+click extends. Asserting on real mouse input catches a regression that the + * selection-math unit test cannot. + */ +@OptIn(ExperimentalTestApi::class) +class ShiftClickPointerInputTest { + + @Test + fun `click then shift+click extends a selection that survives the parallel handlers`() = + runComposeUiTest { + lateinit var state: TextEditorState + setContent { + state = rememberTextEditorState( + initialText = AnnotatedString("The quick brown fox jumps over the lazy dog") + ) + BasicTextEditor( + state = state, + modifier = Modifier.size(width = 400.dp, height = 200.dp), + autoFocus = true, + ) + } + waitForIdle() + + onRoot().performMouseInput { click(Offset(20f, 10f)) } + waitForIdle() + + onRoot().performKeyInput { keyDown(Key.ShiftLeft) } + onRoot().performMouseInput { + moveTo(Offset(220f, 10f)) + press() + release() + } + onRoot().performKeyInput { keyUp(Key.ShiftLeft) } + waitForIdle() + + val selection = state.selector.selection + assertNotNull(selection, "shift+click must leave a selection in place") + assertTrue( + selection.start != selection.end, + "selection must span the gap between the two clicks, not be collapsed by a racing handler", + ) + } + + @Test + fun `shift+click then drag keeps extending the selection`() = runComposeUiTest { + lateinit var state: TextEditorState + setContent { + state = rememberTextEditorState( + initialText = AnnotatedString("The quick brown fox jumps over the lazy dog") + ) + BasicTextEditor( + state = state, + modifier = Modifier.size(width = 400.dp, height = 200.dp), + autoFocus = true, + ) + } + waitForIdle() + + onRoot().performMouseInput { click(Offset(20f, 10f)) } + waitForIdle() + + onRoot().performKeyInput { keyDown(Key.ShiftLeft) } + onRoot().performMouseInput { + moveTo(Offset(120f, 10f)) + press() + moveTo(Offset(240f, 10f)) + release() + } + onRoot().performKeyInput { keyUp(Key.ShiftLeft) } + waitForIdle() + + val selection = state.selector.selection + assertNotNull(selection, "shift+click then drag must leave a selection in place") + assertTrue( + selection.start != selection.end, + "dragging after a shift+click must keep the selection spanning", + ) + } +}