Skip to content

fix: submit composer on numpad enter - #3118

Closed
dso2ng wants to merge 1 commit into
nesquena:masterfrom
dso2ng:fix/numpad-enter-submit
Closed

dso2ng wants to merge 1 commit into
nesquena:masterfrom
dso2ng:fix/numpad-enter-submit

Conversation

@dso2ng

@dso2ng dso2ng commented May 29, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Hermes WebUI already lets users choose whether regular Enter submits or inserts a newline.
  • Full-size keyboards have a separate numeric keypad Enter that many users expect to submit forms.
  • The current composer logic only checks e.key === 'Enter', so Ctrl/Cmd+Enter mode treats both regular Enter and Numpad Enter as newline unless Ctrl/Cmd is held.
  • This PR adds a narrow Numpad Enter detection path so keypad Enter submits while preserving the existing regular Enter/Ctrl+Enter preference behavior and IME guard.

What Changed

  • Added _isNumpadEnter(e) in static/boot.js, using e.code === 'NumpadEnter' with a numpad-location fallback.
  • In Ctrl/Cmd+Enter mode, Numpad Enter now submits without requiring Ctrl/Cmd.
  • Regular Enter still follows the configured send-key preference.
  • Added static regression coverage for the helper, Ctrl/Cmd+Enter branch behavior, and IME guard ordering.
  • Added an Unreleased changelog entry.

Why It Matters

Users who prefer regular Enter for newlines can still submit quickly from a numeric keypad without changing the global send-key setting or breaking IME composition flows.

Verification

  • python -m pytest tests/test_numpad_enter_submit.py tests/test_mobile_layout.py tests/test_ime_composition.py tests/test_issue1443_ime_helper_promotion.py tests/test_1003_preferences_autosave.py -q → 80 passed, 1 warning
  • node --check static/boot.js
  • git diff --check
  • Scanned added diff lines for non-ASCII text; none found.

Risks / Follow-ups

  • This relies on browsers reporting KeyboardEvent.code as NumpadEnter or KeyboardEvent.location as DOM_KEY_LOCATION_NUMPAD. If an unusual remote desktop/browser collapses keypad Enter into regular Enter with no numpad metadata, it will keep the existing regular Enter behavior.
  • No visual UI changes; screenshot evidence is not applicable.

Contract Routing

Task type: small composer keyboard interaction fix.
Touched areas: static/boot.js, composer keyboard contract tests, changelog.
Relevant public docs:

  • AGENTS.md
  • CONTRIBUTING.md
  • docs/CONTRACTS.md
  • docs/UIUX-GUIDE.md
  • DESIGN.md
    Scope boundaries: no layout, theme, streaming, runtime-state, or persistence contract changes.
    Evidence needed before claiming done: targeted keyboard/static tests plus JS syntax and diff hygiene.

Model Used

OpenAI Codex GPT-5.5 via Hermes Agent WebUI, with repository inspection and local shell/test tooling.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Review

Reading static/boot.js:1110-1149 on origin/master against the diff, the change is minimal and lands the right shape. Three points:

Code reference

The patched branch at static/boot.js:1141-1149 (PR HEAD):

if(e.key==='Enter'){
  if(_isImeEnter(e)){return;}
  const isNumpadEnter=_isNumpadEnter(e);
  const _mobileDefault=matchMedia('(pointer:coarse)').matches&&window._sendKey==='enter'&&_isVirtualKeyboardLikelyOpen();
  if(window._sendKey==='ctrl+enter'||_mobileDefault){
    if(isNumpadEnter||e.ctrlKey||e.metaKey){e.preventDefault();send();}
  } else {
    if(!e.shiftKey){e.preventDefault();send();}
  }
}

The IME guard runs first, the numpad detection runs second, and the actual submit is gated inside the ctrl+enter branch only. So if the user is in regular enter mode (default), Numpad Enter still goes through the existing if(!e.shiftKey) path and submits — which is the current behavior anyway, no regression. If the user is in ctrl+enter mode, Numpad Enter now also submits without requiring Ctrl/Cmd. That matches the description.

Points

  1. _isNumpadEnter correctly checks both code and location. Chrome/Firefox/Safari all set e.code === 'NumpadEnter' for the numeric keypad Enter, but the e.location === DOM_KEY_LOCATION_NUMPAD (value 3) fallback covers older Safari and some IME-layered keyboards. Good defensive shape:

    return e.key==='Enter'&&(e.code==='NumpadEnter'||e.location===KeyboardEvent.DOM_KEY_LOCATION_NUMPAD);
  2. No _mobileDefault interaction. A touch device with the virtual keyboard open and _sendKey==='enter' enters the _mobileDefault branch (which treats Enter as newline unless Ctrl/Cmd held). Numpad Enter on such a device is rare but theoretically possible (Bluetooth keypad on tablet). With this PR, Numpad Enter would submit even on touch+virtual-keyboard, bypassing the mobile-default newline behavior. I think that's actually the desired behavior — Numpad Enter on a physical keypad is an explicit submit gesture — but worth calling out so it's not a surprise. The CHANGELOG entry could mention it.

  3. Test approach is fine for a static check, but won't catch behavioral regressions. tests/test_numpad_enter_submit.py grep-checks the source for the helper definition and IME ordering. That's enough to detect accidental deletion of the helper, but it won't catch e.g. someone moving the isNumpadEnter declaration above the IME guard. If you want stronger coverage, a JSDOM-style test that synthesizes a KeyboardEvent({key:'Enter', code:'NumpadEnter'}) and asserts send() was called would be more durable. Not blocking — the static check is consistent with the rest of tests/test_*.py shape.

Cross-repo check

No agent contract touched. static/boot.js keyboard handling is pure WebUI; the agent side has no opinion about Numpad Enter. Nothing to verify in ~/.hermes/hermes-agent/.

Verdict

LGTM. Small surface, targeted fix, IME-safe ordering, location fallback for browser quirks. The only nit is calling out the touch+virtualKB+numpad-bluetooth edge case in the CHANGELOG entry so users on tablets with external keyboards understand the change. Ready to merge as-is.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.159 (Release EE, stage-batch41) via release PR #3144 — thank you, @dso2ng! 🎉

Your change (Numpad Enter submit shortcut) is now on master. Your authorship is preserved via the --no-ff merge of your commits in the release branch.

Two PRs in this batch needed a small semantic merge against code that landed in v0.51.158:

GitHub didn't auto-close this PR because the release merged resolved/reparented commits rather than your branch's exact head SHA, so closing manually. The full diff and tests are verified present on master. Full sequential pytest passed (6810 tests). Closing as shipped.

SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
# Conflicts:
#	CHANGELOG.md
#	static/boot.js
bernyforce pushed a commit to bernyforce/hermes-webui that referenced this pull request Jul 29, 2026
# Conflicts:
#	CHANGELOG.md
#	static/boot.js
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