From 6d13c792699770920cdf68676d80d8c9a16220c0 Mon Sep 17 00:00:00 2001 From: Sunny Chung Date: Mon, 4 Nov 2024 00:17:24 +0800 Subject: [PATCH] fix undo/redo in BigMonospaceText should restore selection as well --- .../hellohttp/ux/bigtext/BigMonospaceText.kt | 84 ++++++++++++------- .../hellohttp/ux/bigtext/BigTextImpl.kt | 49 ++++++++--- .../ux/bigtext/BigTextInputChangeOperation.kt | 12 ++- .../ux/bigtext/BigTextUndoMetadata.kt | 6 ++ .../hellohttp/ux/bigtext/BigTextViewState.kt | 8 +- .../test/bigtext/BigTextUndoRedoTest.kt | 16 ++-- 6 files changed, 120 insertions(+), 55 deletions(-) create mode 100644 src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextUndoMetadata.kt diff --git a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigMonospaceText.kt b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigMonospaceText.kt index ba8b72bb..3a317c97 100644 --- a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigMonospaceText.kt +++ b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigMonospaceText.kt @@ -299,6 +299,15 @@ private fun CoreBigMonospaceText( // log.v { "text = |${text.buildString()}|" } // log.v { "transformedText = |${transformedText.buildString()}|" } + remember(text, viewState) { + text.undoMetadataSupplier = { + BigTextUndoMetadata( + cursor = viewState.cursorIndex, + selection = viewState.selection, + ) + } + } + fun fireOnLayout() { lineHeight = (textLayouter.charMeasurer as ComposeUnicodeCharMeasurer).getRowHeight() onTextLayout?.let { callback -> @@ -455,6 +464,26 @@ private fun CoreBigMonospaceText( ) } + fun scrollToCursor() { + val layoutResult = layoutResult ?: return + + // scroll to cursor position if out of visible range + val visibleVerticalRange = scrollState.value .. scrollState.value + height + val row = transformedText.findRowIndexByPosition(viewState.transformedCursorIndex) + val rowVerticalRange = layoutResult.getTopOfRow(row).toInt() .. layoutResult.getBottomOfRow(row).toInt() + if (rowVerticalRange !in visibleVerticalRange) { + val scrollToPosition = if (rowVerticalRange.start < visibleVerticalRange.start) { + rowVerticalRange.start + } else { + // scroll to a position that includes the bottom of the row + a little space + minOf(layoutResult.bottom.toInt(), maxOf(0, rowVerticalRange.endInclusive + maxOf(2, (layoutResult.rowHeight * 0.5).toInt()) - height)) + } + coroutineScope.launch { + scrollState.animateScrollTo(scrollToPosition) + } + } + } + fun updateViewState() { viewState.lastVisibleRow = minOf(viewState.lastVisibleRow, transformedText.lastRowIndex) log.d { "lastVisibleRow = ${viewState.lastVisibleRow}, lastRowIndex = ${transformedText.lastRowIndex}" } @@ -498,15 +527,16 @@ private fun CoreBigMonospaceText( val start = viewState.selection.start val endExclusive = viewState.selection.endInclusive + 1 delete(start, endExclusive) - if (isSaveUndoSnapshot) { - text.recordCurrentChangeSequenceIntoUndoHistory() - } viewState.selection = EMPTY_SELECTION_RANGE // cannot use IntRange.EMPTY as `viewState.selection.start` is in use viewState.transformedSelection = EMPTY_SELECTION_RANGE viewState.cursorIndex = start viewState.updateTransformedCursorIndexByOriginal(transformedText) viewState.transformedSelectionStart = viewState.transformedCursorIndex + + if (isSaveUndoSnapshot) { + text.recordCurrentChangeSequenceIntoUndoHistory() + } } } @@ -524,9 +554,6 @@ private fun CoreBigMonospaceText( } val insertPos = viewState.cursorIndex insertAt(insertPos, textInput) - if (isSaveUndoSnapshot) { - text.recordCurrentChangeSequenceIntoUndoHistory() - } updateViewState() if (log.config.minSeverity <= Severity.Verbose) { (transformedText as BigTextImpl).printDebug("transformedText onType '${textInput.string().replace("\n", "\\n")}'") @@ -536,6 +563,9 @@ private fun CoreBigMonospaceText( viewState.updateTransformedCursorIndexByOriginal(transformedText) viewState.transformedSelectionStart = viewState.transformedCursorIndex log.v { "set cursor pos 2 => ${viewState.cursorIndex} t ${viewState.transformedCursorIndex}" } + if (isSaveUndoSnapshot) { + text.recordCurrentChangeSequenceIntoUndoHistory() + } } fun onDelete(direction: TextFBDirection): Boolean { @@ -552,12 +582,12 @@ private fun CoreBigMonospaceText( if (cursor + 1 <= text.length) { onValuePreChange(BigTextChangeEventType.Delete, cursor, cursor + 1) text.delete(cursor, cursor + 1) - text.recordCurrentChangeSequenceIntoUndoHistory() onValuePostChange(BigTextChangeEventType.Delete, cursor, cursor + 1) updateViewState() if (log.config.minSeverity <= Severity.Verbose) { (transformedText as BigTextImpl).printDebug("transformedText onDelete $direction") } + text.recordCurrentChangeSequenceIntoUndoHistory() return true } } @@ -565,7 +595,6 @@ private fun CoreBigMonospaceText( if (cursor - 1 >= 0) { onValuePreChange(BigTextChangeEventType.Delete, cursor - 1, cursor) text.delete(cursor - 1, cursor) - text.recordCurrentChangeSequenceIntoUndoHistory() onValuePostChange(BigTextChangeEventType.Delete, cursor - 1, cursor) updateViewState() if (log.config.minSeverity <= Severity.Verbose) { @@ -576,6 +605,7 @@ private fun CoreBigMonospaceText( viewState.updateTransformedCursorIndexByOriginal(transformedText) viewState.transformedSelectionStart = viewState.transformedCursorIndex log.v { "set cursor pos 3 => ${viewState.cursorIndex} t ${viewState.transformedCursorIndex}" } + text.recordCurrentChangeSequenceIntoUndoHistory() return true } } @@ -583,9 +613,9 @@ private fun CoreBigMonospaceText( return false } - fun onUndoRedo(operation: (BigTextChangeCallback) -> Unit) { + fun onUndoRedo(operation: (BigTextChangeCallback) -> Pair) { var lastChangeEnd = -1 - operation(object : BigTextChangeCallback { + val stateToBeRestored = operation(object : BigTextChangeCallback { override fun onValuePreChange( eventType: BigTextChangeEventType, changeStartIndex: Int, @@ -606,7 +636,17 @@ private fun CoreBigMonospaceText( } } }) - if (lastChangeEnd >= 0) { + updateViewState() + (stateToBeRestored.second as? BigTextUndoMetadata)?.let { state -> + viewState.selection = state.selection + viewState.updateTransformedSelectionBySelection(transformedText) + viewState.cursorIndex = state.cursor + viewState.updateTransformedCursorIndexByOriginal(transformedText) + viewState.transformedSelectionStart = viewState.transformedCursorIndex + scrollToCursor() + return + } + if (lastChangeEnd >= 0) { // this `if` should never execute viewState.cursorIndex = lastChangeEnd viewState.updateTransformedCursorIndexByOriginal(transformedText) viewState.transformedSelectionStart = viewState.transformedCursorIndex @@ -690,29 +730,9 @@ private fun CoreBigMonospaceText( return viewState.cursorIndex + wordBoundaryAt } - fun scrollToCursor() { - val layoutResult = layoutResult ?: return - - // scroll to cursor position if out of visible range - val visibleVerticalRange = scrollState.value .. scrollState.value + height - val row = transformedText.findRowIndexByPosition(viewState.transformedCursorIndex) - val rowVerticalRange = layoutResult.getTopOfRow(row).toInt() .. layoutResult.getBottomOfRow(row).toInt() - if (rowVerticalRange !in visibleVerticalRange) { - val scrollToPosition = if (rowVerticalRange.start < visibleVerticalRange.start) { - rowVerticalRange.start - } else { - // scroll to a position that includes the bottom of the row + a little space - minOf(layoutResult.bottom.toInt(), maxOf(0, rowVerticalRange.endInclusive + maxOf(2, (layoutResult.rowHeight * 0.5).toInt()) - height)) - } - coroutineScope.launch { - scrollState.animateScrollTo(scrollToPosition) - } - } - } - fun updateOriginalCursorOrSelection(newPosition: Int, isSelection: Boolean) { val oldCursorPosition = viewState.cursorIndex - viewState.cursorIndex = newPosition // TODO scroll to new position + viewState.cursorIndex = newPosition viewState.updateTransformedCursorIndexByOriginal(transformedText) if (isSelection) { val selectionStart = if (viewState.hasSelection()) { diff --git a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextImpl.kt b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextImpl.kt index 703c0bba..457af955 100644 --- a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextImpl.kt +++ b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextImpl.kt @@ -71,7 +71,12 @@ open class BigTextImpl( var decorator: BigTextDecorator? = null + var undoMetadataSupplier: (() -> Any?)? = null + + private var currentUndoMetadata: Any? = null + private var currentRedoMetadata: Any? = null var currentChanges: MutableList = mutableListOf() + private set val undoHistory = CircularList(undoHistoryCapacity) val redoHistory = CircularList(undoHistoryCapacity) @@ -419,6 +424,7 @@ open class BigTextImpl( leftStringLength = 0 } if (isUndoEnabled) { + recordCurrentUndoMetadata() currentChanges += BigTextInputChange( type = BigTextChangeEventType.Insert, buffer = buffer, @@ -957,6 +963,7 @@ open class BigTextImpl( isD = true } if (isUndoEnabled) { + recordCurrentUndoMetadata() currentChanges += BigTextInputChange( type = BigTextChangeEventType.Delete, buffer = node.value.buffer, @@ -1031,9 +1038,18 @@ open class BigTextImpl( } if (currentChanges.isNotEmpty()) { - undoHistory.push(BigTextInputOperation(currentChanges.toList())) + undoHistory.push(BigTextInputOperation(currentChanges.toList(), currentUndoMetadata, undoMetadataSupplier?.invoke())) currentChanges = mutableListOf() + recordCurrentUndoMetadata() + } + } + + protected fun recordCurrentUndoMetadata() { + if (currentChanges.isEmpty()) { + currentUndoMetadata = undoMetadataSupplier?.invoke() } + currentRedoMetadata = undoMetadataSupplier?.invoke() + log.d { "reset um = $currentUndoMetadata, rm = $currentRedoMetadata" } } protected fun clearRedoHistory() { @@ -1110,33 +1126,42 @@ open class BigTextImpl( } } - fun undo(callback: BigTextChangeCallback? = null): Boolean { + fun undo(callback: BigTextChangeCallback? = null): Pair { if (!isUndoEnabled) { - return false + return false to null } if (currentChanges.isNotEmpty()) { + val undoMetadata = currentUndoMetadata + val redoMetadata = currentRedoMetadata applyReverseChangeSequence(currentChanges, callback) - redoHistory.push(BigTextInputOperation(currentChanges.toList())) + redoHistory.push(BigTextInputOperation(currentChanges.toList(), undoMetadata, redoMetadata)) currentChanges = mutableListOf() - return true + recordCurrentUndoMetadata() + return true to undoMetadata } - val lastOperation = undoHistory.removeHead() ?: return false + val lastOperation = undoHistory.removeHead() ?: return false to null applyReverseChangeSequence(lastOperation.changes, callback) + currentUndoMetadata = lastOperation.undoMetadata + currentRedoMetadata = lastOperation.redoMetadata + log.d { "undo set um = $currentUndoMetadata, rm = $currentRedoMetadata" } redoHistory.push(lastOperation) - return true + return true to lastOperation.undoMetadata } - fun redo(callback: BigTextChangeCallback? = null): Boolean { + fun redo(callback: BigTextChangeCallback? = null): Pair { if (!isUndoEnabled) { - return false + return false to null } if (currentChanges.isNotEmpty()) { // should not happen - return false + return false to null } - val lastOperation = redoHistory.removeHead() ?: return false + val lastOperation = redoHistory.removeHead() ?: return false to null applyChangeSequence(lastOperation.changes, callback) + currentUndoMetadata = lastOperation.undoMetadata + currentRedoMetadata = lastOperation.redoMetadata + log.d { "undo set um = $currentUndoMetadata, rm = $currentRedoMetadata" } undoHistory.push(lastOperation) - return true + return true to lastOperation.redoMetadata } fun isUndoable(): Boolean = isUndoEnabled && (currentChanges.isNotEmpty() || undoHistory.isNotEmpty) diff --git a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextInputChangeOperation.kt b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextInputChangeOperation.kt index 47be6a73..dae9542e 100644 --- a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextInputChangeOperation.kt +++ b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextInputChangeOperation.kt @@ -3,7 +3,17 @@ package com.sunnychung.application.multiplatform.hellohttp.ux.bigtext import com.sunnychung.application.multiplatform.hellohttp.extension.length data class BigTextInputOperation( - val changes: List + val changes: List, + + /** + * Metadata to apply if changes of this operation have been reversed. + */ + val undoMetadata: Any?, + + /** + * Metadata to apply if changes of this operation have been applied. + */ + val redoMetadata: Any?, ) data class BigTextInputChange( diff --git a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextUndoMetadata.kt b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextUndoMetadata.kt new file mode 100644 index 00000000..dabb1f1f --- /dev/null +++ b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextUndoMetadata.kt @@ -0,0 +1,6 @@ +package com.sunnychung.application.multiplatform.hellohttp.ux.bigtext + +data class BigTextUndoMetadata( + val cursor: Int, + val selection: IntRange, +) diff --git a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextViewState.kt b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextViewState.kt index db97a54a..7d1e372e 100644 --- a/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextViewState.kt +++ b/src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextViewState.kt @@ -50,8 +50,12 @@ class BigTextViewState { } internal fun updateTransformedSelectionBySelection(transformedText: BigTextTransformed) { - transformedSelection = transformedText.findTransformedPositionByOriginalPosition(selection.first) .. - transformedText.findTransformedPositionByOriginalPosition(selection.last) + transformedSelection = if (!selection.isEmpty()) { + transformedText.findTransformedPositionByOriginalPosition(selection.first) .. + transformedText.findTransformedPositionByOriginalPosition(selection.last) + } else { + IntRange.EMPTY + } } internal var transformedCursorIndex by mutableStateOf(0) diff --git a/src/jvmTest/kotlin/com/sunnychung/application/multiplatform/hellohttp/test/bigtext/BigTextUndoRedoTest.kt b/src/jvmTest/kotlin/com/sunnychung/application/multiplatform/hellohttp/test/bigtext/BigTextUndoRedoTest.kt index 21db5622..76f93eca 100644 --- a/src/jvmTest/kotlin/com/sunnychung/application/multiplatform/hellohttp/test/bigtext/BigTextUndoRedoTest.kt +++ b/src/jvmTest/kotlin/com/sunnychung/application/multiplatform/hellohttp/test/bigtext/BigTextUndoRedoTest.kt @@ -121,7 +121,7 @@ class BigTextUndoRedoTest { t.delete(6 .. 6) assertEquals("abcdef", t.buildString()) listOf("abcdefg", "abcd").forEach { expected -> - assertEquals(true, t.undo()) + assertEquals(true, t.undo().first) assertEquals(expected, t.buildString()) } @@ -129,7 +129,7 @@ class BigTextUndoRedoTest { t.append("x") assertEquals("abcdx", t.buildString()) (1..10).forEach { - assertEquals(false, t.redo()) + assertEquals(false, t.redo().first) assertEquals("abcdx", t.buildString()) } @@ -139,27 +139,27 @@ class BigTextUndoRedoTest { fun assertUndoRedoUndo(reversedExpectedStrings: List, t: BigTextImpl) { assertEquals(reversedExpectedStrings.first(), t.buildString()) reversedExpectedStrings.stream().skip(1).forEach { expected -> - assertEquals(true, t.undo()) + assertEquals(true, t.undo().first) assertEquals(expected, t.buildString()) } (1..3).forEach { - assertEquals(false, t.undo()) + assertEquals(false, t.undo().first) assertEquals(reversedExpectedStrings.last(), t.buildString()) } reversedExpectedStrings.asReversed().stream().skip(1).forEach { expected -> - assertEquals(true, t.redo()) + assertEquals(true, t.redo().first) assertEquals(expected, t.buildString()) } (1..10).forEach { - assertEquals(false, t.redo()) + assertEquals(false, t.redo().first) assertEquals(reversedExpectedStrings.first(), t.buildString()) } reversedExpectedStrings.stream().skip(1).forEach { expected -> - assertEquals(true, t.undo()) + assertEquals(true, t.undo().first) assertEquals(expected, t.buildString()) } (1..10).forEach { - assertEquals(false, t.undo()) + assertEquals(false, t.undo().first) assertEquals(reversedExpectedStrings.last(), t.buildString()) } }