🐛 Fix TUI focus navigation requiring Tab/arrow twice (#2) #14

Open
ossfixx4756 wants to merge 1 commit from ossfixx4756/sshception:fix/2-tui-focus-navigation-twice into dev
First-time contributor

Bug

Closes #2

On the TUI home screen, moving focus to the next action could require pressing Tab / arrow-down twice: the first keypress appeared to do nothing, and only the second press visibly moved focus. This was especially visible after interacting with the host-selection dialog.

Root cause

Three focus-management issues in src/tui/views/home.js, around reblessed's (the blessed fork used here) Form widget:

  1. Escape stranding focus on a non-actionable container. Form registers a bubbling element keypress handler that maps Escape to form.focus() (see node_modules/blessed/lib/widgets/form.js). After the host actions dialog was open, Escape moved focus onto its invisible form container, and the next Tab/Down was first consumed moving from that container onto a child action - the first keypress was effectively lost. The same Escape-to-container strand existed on the main actions form.
  2. Stale focus cursor across dialog reopen. The host actions dialog is a single reused Form. Explicitly focusing the Cancel button did not reset its internal _selected cursor, so after any Tab/arrow traversal, reopening the dialog and pressing Tab once could initially move focus back to the remembered child (often the already-focused Cancel) instead of to Connect - another first keypress that did not advance.
  3. Accumulating Escape listeners. Every press of "Select host" did emitter.on('escape', ...) and never removed it. The dashboard remounts the whole home view on each navigation cycle using the same session-wide emitter (see src/tui/controllers/dashboard.js _home()), so stale closures piled up and could keep stealing Escape.

Solution

  • Escaping the host actions dialog now cancels it deterministically (hide it, reset the Remove confirm label) and returns focus to the host list - the same behavior as the Cancel button. A late element keypress listener (registered after the Form's built-in one, so it runs last) overrides the container self-focus.
  • Escaping the main actions returns to the primary action ("Select host" / "Add host" when there are no hosts) instead of the form container.
  • Escaping out of the host list is bound on the list element itself, i.e. its lifecycle follows the per-mount widgets rather than the shared session-wide emitter, so no global listener can leak or target a stale view.
  • resetSelected() is called before focusing Cancel when a host row is opened, so the first Tab always lands on Connect.

Verification

  • Added node-based regression tests that drive the real TUI view through a virtual terminal input stream (no SSH connection or external services needed): yarn test -> 6 passing.
  • Scenarios: dialog Escape cancels in one step; Tab x4 wraps through actions with no no-op press, Shift-Tab/Up/Down one-press each, verified across a second dialog reopen; Escape from the host list restores "Select host"; repeated view remounts create zero global Escape subscriptions; navigation never rests on a form container; hidden dialog never steals focus; no-hosts layout stays 1:1.
  • node --check on the changed JS files.

Scope / notes

  • Change is limited to the TUI home view, the accompanying regression tests, and a yarn test npm script. CLI/connect/account paths are untouched.
  • #12 (cursor not restored after disconnecting from a host) was considered but intentionally left for a separate PR to keep this one focused.
## Bug Closes #2 On the TUI home screen, moving focus to the next action could require pressing Tab / arrow-down twice: the first keypress appeared to do nothing, and only the second press visibly moved focus. This was especially visible after interacting with the host-selection dialog. ## Root cause Three focus-management issues in `src/tui/views/home.js`, around reblessed's (the `blessed` fork used here) `Form` widget: 1. **Escape stranding focus on a non-actionable container.** `Form` registers a bubbling `element keypress` handler that maps Escape to `form.focus()` (see `node_modules/blessed/lib/widgets/form.js`). After the host actions dialog was open, Escape moved focus onto its invisible form container, and the next Tab/Down was first consumed moving from that container onto a child action - the first keypress was effectively lost. The same Escape-to-container strand existed on the main actions form. 2. **Stale focus cursor across dialog reopen.** The host actions dialog is a single reused Form. Explicitly focusing the Cancel button did not reset its internal `_selected` cursor, so after any Tab/arrow traversal, reopening the dialog and pressing Tab once could initially move focus *back to the remembered child* (often the already-focused Cancel) instead of to Connect - another first keypress that did not advance. 3. **Accumulating Escape listeners.** Every press of "Select host" did `emitter.on('escape', ...)` and never removed it. The dashboard remounts the whole home view on each navigation cycle using the same session-wide emitter (see `src/tui/controllers/dashboard.js` `_home()`), so stale closures piled up and could keep stealing Escape. ## Solution - Escaping the host actions dialog now cancels it deterministically (hide it, reset the Remove confirm label) and returns focus to the host list - the same behavior as the Cancel button. A late `element keypress` listener (registered after the Form's built-in one, so it runs last) overrides the container self-focus. - Escaping the main actions returns to the primary action ("Select host" / "Add host" when there are no hosts) instead of the form container. - Escaping out of the host list is bound on the list element itself, i.e. its lifecycle follows the per-mount widgets rather than the shared session-wide emitter, so no global listener can leak or target a stale view. - `resetSelected()` is called before focusing Cancel when a host row is opened, so the first Tab always lands on Connect. ## Verification - Added node-based regression tests that drive the real TUI view through a virtual terminal input stream (no SSH connection or external services needed): `yarn test` -> 6 passing. - Scenarios: dialog Escape cancels in one step; Tab x4 wraps through actions with no no-op press, Shift-Tab/Up/Down one-press each, verified across a second dialog reopen; Escape from the host list restores "Select host"; repeated view remounts create zero global Escape subscriptions; navigation never rests on a form container; hidden dialog never steals focus; no-hosts layout stays 1:1. - `node --check` on the changed JS files. ## Scope / notes - Change is limited to the TUI home view, the accompanying regression tests, and a `yarn test` npm script. CLI/connect/account paths are untouched. - #12 (cursor not restored after disconnecting from a host) was considered but intentionally left for a separate PR to keep this one focused.
ossfixx4756 added 1 commit 2026-10-28 01:29:00 +01:00
Navigation could require an extra Tab/Down because focus sometimes rested on a reblessed Form container (non-actionable):

- escaping the selected-host dialog now cancels it and returns focus to the host list, and escaping main actions returns to the primary action;
- the dialog's internal Form selection cursor is reset on every reopen so the first Tab lands on the next action instead of being a no-op;
- Escape is bound per view mount instead of subscribing once per 'Select host' press on the session-wide emitter (listeners accumulated and kept stealing focus with stale closures);
- add node-based regression tests for the focus flows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'fix/2-tui-focus-navigation-twice:ossfixx4756-fix/2-tui-focus-navigation-twice'
git checkout 'ossfixx4756-fix/2-tui-focus-navigation-twice'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout 'dev'
git merge --no-ff 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git checkout 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git rebase 'dev'
git checkout 'dev'
git merge --ff-only 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git checkout 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git rebase 'dev'
git checkout 'dev'
git merge --no-ff 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git checkout 'dev'
git merge --squash 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git checkout 'dev'
git merge --ff-only 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git checkout 'dev'
git merge 'ossfixx4756-fix/2-tui-focus-navigation-twice'
git push origin 'dev'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: KaKi87/sshception#14
No description provided.