feat(toolbar): UNDO_WORD key — revert last commit to alternatives (#35)#66
Merged
Conversation
…#35) Adds an Undo-word toolbar key that reverts the last committed (gesture) word back to its alternatives in the suggestion strip, modeled on the JOIN_NEXT toolbar key. - New KeyCode.UNDO_WORD (-251, next free toolbar code); ToolbarKey.UNDO_WORD + getCodeForToolbarKey mapping; ic_undo / ic_undo_rounded icons; talkback string. - InputLogic.handleUndoWord: guarded by mLastComposedWord.canRevertCommit(), reuses the backspace revert sequence (revertCommit + restartSuggestionsOnWordTouchedByCursor) so the committed word's SuggestionSpans repopulate the strip with its alternatives. Scope: toolbar key only. The swipe-up/down assignable target (also in #35) is deferred until the shortcut-assignment plumbing (shared with #37) exists. Needs on-device verification that gesture-committed words restore their alternatives (vs dictionary-fresh suggestions) — see PR notes.
On-device testing found the revertCommit reuse was wrong: it deletes committedWord.length()+separator before the CURRENT cursor, but after a gesture the trailing space is a pending PHANTOM space (not in the text), so it over-deleted into the previous word; on stale/empty state it deleted the wrong span -> garbage. Drop revertCommit entirely. handleUndoWord now just calls restartSuggestionsOnWordTouchedByCursor: after a gesture the cursor sits right after the committed word (phantom space not in text), so that word is re-opened for re-selection with its alternatives shown — exactly like tapping a word to re-edit it. Non-destructive (no delete -> no garbage, no over-delete), and its existing guards no-op safely when the cursor isn't on a word / has selection / no-spaces language / suggestions disabled. Also: distinct icon — UNDO_WORD now uses ic_edit (UNDO already uses ic_undo).
Owner
Author
|
Reworked after your on-device testing. Root cause of the bugs: it reused backspace's |
…sh lookup (#35) restartSuggestionsOnWordTouchedByCursor reads suggestion spans from the EDITOR text, but gesture commits don't leave them there, so it fell into a fresh typing-style lookup -> nonsense suggestions. Read the alternatives from mLastComposedWord.mCommittedWord's in-memory SuggestionSpan instead (the actual gesture/autocorrect candidates the original revertCommit used). Still non-destructive: verify the committed word is exactly before the cursor, set the composing region over it (no delete), and show those alternatives; no-op if the word moved/changed or has no real alternatives.
…space (#35) The real value is AFTER you've pressed space (the strip already shows alternatives right after a word). mLastComposedWord.deactivate() runs on the space keypress, but the object + its in-memory SuggestionSpan alternatives survive. So handleUndoWord now finds the committed word before the cursor allowing a trailing whitespace separator, moves the cursor back into it (setSelection), re-composes it, and shows its original alternatives. Non-destructive (no deletion). No-ops if the last word before the cursor isn't the committed one or it has no alternatives.
…#35) Root cause of the no-ops: commitChosenWord stores the PLAIN chosenWord as mLastComposedWord.mCommittedWord (the spans-carrying string only goes to the editor at commitText), so reading SuggestionSpans off mCommittedWord always found none -> size<=1 -> no-op. The candidates actually live in the editor text's SuggestionSpans. So: move the cursor to the end of the last word before the cursor (skip a trailing space), then postResumeSuggestions(delay) -> MSG_RESUME_SUGGESTIONS -> restartSuggestionsOnWordTouchedByCursor, which reads the word's editor spans after the cursor settles. Non-destructive, race-free (uses the standard resume mechanism).
Owner
Author
|
On-device verified (S24+): after a word + space, Undo word jumps back to the word and shows its real alternatives. (Root cause was that the spans live only in the editor text, not mLastComposedWord.) ✅ |
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
1 participant
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.
Implements the toolbar-key half of #35 (A10).
What
An Undo-word toolbar key that reverts the last committed (gesture) word straight back to its alternatives in the suggestion strip — modeled end-to-end on the existing JOIN_NEXT toolbar key.
KeyCode.UNDO_WORD = -251(next free toolbar code after JOIN_NEXT/-249, FORCE_NEXT_SPACE/-250) + added to the working-codes list.ToolbarKey.UNDO_WORD+getCodeForToolbarKeymapping;ic_undo/ic_undo_roundedicons; talkback description.InputLogic.handleUndoWord: guarded bymLastComposedWord.canRevertCommit(), then reuses the backspace revert sequence (revertCommit+restartSuggestionsOnWordTouchedByCursor).revertCommitre-commits the original word with its SuggestionSpans; the restart reads those spans and repopulates the strip with the alternatives. For a gesture commitdidCommitTypedWord()is false, socanRevertCommit()is true.Scope
Toolbar key only. The swipe-up/down assignable target (also named in #35) is deferred — it depends on the shortcut-assignment plumbing (shared with #37) that doesn't exist yet.
Verification
:app:testOfflineRunTestsUnitTest= 228 tests, 4 failed (all the Windows-only ParserTest set; pass on Linux CI) — zero new failures.Implemented by a subagent; coordinator fixed a private-field access (
mActive→canRevertCommit()), compiled, and ran the suite. Basedev, leaving for review.