Repository navigation
Conversation
Contributor
Author
|
@LeanBitLab Can you test this, or make test apk just for me to test it? You don't have to merge it for next version till it's tested, i want to polish this delete gesture as my last request (it shouldn't be broken like it was back then at version 429-7 , but will test it just in case). This should also solve all those previous issues i found while using your 2D solution. Sorry for troubling you with this same issue and thank you |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: LeanType issue #467. Based on current
main(same source as beta-429-9).Problem
Swiping up on the delete key selects "lines" using
getLineBoundariesBackwards(), whichdoes not know the real layout of the text field: it splits on
\nand cuts longersegments into chunks of roughly 40-50 characters. These guessed boundaries rarely match the
real soft-wrapped lines, so after an upward step the selection starts at an arbitrary word
somewhere in the previous line.
An earlier attempt (beta-429-7) asked the editor to do the movement with Shift+DPAD_UP on a
reversed selection (anchor = right end, extent = left end). The recording attached to the issue
shows it broken: the selection would not go further than the second line above, and it flickered
in that line when swiping up again. That approach was reverted in beta-429-8.
Likely cause (found by reading the code, not confirmed on a device): an editor reports a reversed
selection to the IME as
selStart = anchor,selEnd = extent, sonewSelStart > newSelEnd.InputLogic.onUpdateSelection()passes these values toresetCachesUponCursorMoveAndReturnSuccess(), which stores them inmExpectedSelStart/Endwithout normalising. The delete swipe then sees
deleteSwipeInitialEnd != expectedSelectionEndat the next move event, re-initialises its state from the wrong values and the selection jumps back,
which looks like a stall plus flicker. This change never creates a reversed selection.
Approach
Let the editor find the position one visual line above, but with plain events and without
changing the selection logic:
send a plain DPAD_UP key event (the same events the toolbar arrow keys already send) and read
the new caret position back with
getExtractedText(synchronous).deleteSwipeLineBoundaries(index 0 = where the swipe started,index n = left end after n upward steps, same column as the arrow key gives).
up and down always return to exactly the same positions and never go below the original
start.
part (
deleteSwipeCharOffset/deleteSwipeWordIndex) relative to the entry for the current line,so up + left (diagonal) simply adds characters/words left of the exact position above.
beginBatchEdit()/endBatchEdit(), so the editorreports only the final selection to the IME. The temporary selections are always collapsed or in
normal order (start <= end), never reversed.
Everything else already in main keeps working unchanged: right-swipe deselection after a vertical
swipe (
deleteSwipeSelectionAnchor()is stillboundaries[0], the swipe start), the word-modeforward boundaries, and the thumb-drift filter in
PointerTracker.ktfor a right swipe after a left swipe(
PointerTracker.ktis not modified).Changes
KeyboardActionListenerImpl.ktdeleteSwipeApproxLineBoundaries,deleteSwipeUseEditorLines,deleteSwipeEditorTopReached(initialised when a swipe starts, cleared in
onUpWithDeletePointerActive()andresetMetaState()).onMoveDeletePointer()extends the boundaries for upward steps before the existing char/word code runs.extendDeleteSwipeLinesByEditor()andeditorPositionOneLineUp().extendLineBoundaries(boundaries, targetIndex, oneLineUp)(pure, unit tested) andLineExtendResult { REACHED, TOP, FAILED }.RichInputConnection.kt: newgetEditorSelection()(synchronousgetExtractedText, asks only for awindow of 10240 chars so large fields are not copied completely).
WordDeleteSwipeTest.kt: 5 new tests forextendLineBoundarieswith a fake editor (extends one line at atime, does not call the editor if the entry exists, stops at the top of the text, reports failure,
keeps entries found before a failure).
Fallbacks
approximation (an up arrow in a single-line field could move focus).
boundaries (
getLineBoundariesBackwards) are used for the rest of that swipe.Not verified
Not compiled and not run (no Android toolchain available to me). Please test on a device in:
a native EditText, a web textarea (Chrome/Brave), and a field with very long soft-wrapped lines with no spaces
(like the one in the beta-429-7 recording), each in char mode and word-by-word mode: