Skip to content

feat(core): add an enter key to the mnemonic keyboard - #2699

Merged
ethicnology merged 1 commit into
developfrom
feat/mnemonic-keyboard-improvements
Aug 20, 2026
Merged

feat(core): add an enter key to the mnemonic keyboard#2699
ethicnology merged 1 commit into
developfrom
feat/mnemonic-keyboard-improvements

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

Focus stayed in the field you'd just finished, so every word cost an extra tap to reach the next one. This adds an Enter key that moves focus forward.

Field state Enter
Complete word Moves to the next field
Empty Moves on, so a gap can be skipped
Partial word Disabled — it would leave a fragment that looks like a finished word
Last field Dismisses the keyboard, matching what a chip tap already does
Screen.Recording.2026-08-20.at.1.52.21.PM.mov

@wired-pasteque wired-pasteque self-assigned this Aug 20, 2026
@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 3m 8s —— View job


Review: Enter key for the mnemonic keyboard

  • Read the diff against origin/develop
  • Read surrounding code for context (keyboard + widget)
  • Analyze for correctness bugs, security issues, regressions
  • Post findings and summary (inline comments not permitted — see note below)

Verdict: LGTM. I found no correctness bugs, security issues, or regressions — only two minor nits. Nice PR: small, well-documented, and the tests cover all four rows of the behavior table (advance on whole word, skip empty, hold on prefix, dismiss on last field).

What I checked

  • Race safety_onKeyEnter (lib/core/widgets/mnemonic_widget.dart:843) re-checks _canLeaveField against the live controller text instead of trusting the build-time canAdvance flag, matching the existing _onKeyLetter pattern. A tap racing the frame that disables the key is correctly dropped. ✅
  • Never writes to the sentence — the enter path only moves focus (_focusNext / unfocus); it can't auto-accept a suggestion, which matters in paranoid mode where the first chip is arbitrary. The whole-word check runs against the full wordlist rather than the checksum candidates, consistent with the transcription-error rationale in _maybeAutoFill. ✅
  • Paranoid-mode invariant holds — enter doesn't reshuffle, but it doesn't need to: reshuffles happen before each letter write, so each displayed layout still receives at most one letter tap, and the enter key sits in a fixed (unshuffled) slot that leaks nothing. ✅
  • Focus/keyboard state — moving focus fires _onFocusChanged_refreshHint, so enabledLetters/canBackspace/canAdvance rebuild correctly for the new field. Last-field unfocus hides the keyboard bar via _activeField. Edge cases (single field, _activeField == null) are handled. ✅
  • Layout math — the new trailingKeys flex is correct: middle row is now 9 letters + 1 = 10 units, bottom row 7 letters + 2 = 9 units; the two trailing Expandeds split their slot evenly. No RangeError risk; the 26-key assert is untouched. ✅
  • Semantics — the enter key not being wrapped in ExcludeSemantics is fine: unlike letters, it reveals nothing about the seed, consistent with backspace/shuffle. ✅
  • Call sitesMnemonicKeyboard is only constructed in mnemonic_widget.dart and its tests; both were updated for the new required params. ✅

Nits (non-blocking)

  1. Stale doc commentlib/core/widgets/mnemonic_keyboard.dart:51: "Paranoid mode toggle, shown as a key next to backspace" — backspace moved to the middle row in this PR, so shuffle now sits next to enter. Fix this →
  2. Slight key-width asymmetry — rows now total 10/10/9 flex units, so bottom-row keys render ~11% wider than the rest (before this PR it was 10/9/9, with the middle row wider). Purely cosmetic and arguably an improvement in consistency; just flagging that "trailing keys stay a letter wide" (mnemonic_keyboard.dart:168) is only true within a row.

Notes on process

  • I couldn't post these as inline review comments: the gh api call to create PR review comments isn't in my allowed tools for this run. Findings are anchored with file:line references above instead.
  • Flutter isn't installed in this runner, so I couldn't execute the new tests; I reviewed them statically.

@ethicnology
ethicnology merged commit b3b9f4e into develop Aug 20, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants