🐛 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
3 changed files with 277 additions and 7 deletions

View file

@ -30,7 +30,8 @@
},
"scripts": {
"keygen": "ssh-keygen -f host.rsa.key -N '' && ssh-keygen -t ed25519 -f host.ed25519.key -N '' && rm *.pub",
"start": "node main.js"
"start": "node main.js",
"test": "node --test tests/"
},
"type": "module"
}

View file

@ -13,12 +13,12 @@ export default (
) => ({
hosts,
callback,
isTotpEnabled,
emitter
isTotpEnabled
}) => {
let n = 3;
const
mainActionsFormContainer = flexContainer(form(screen)),
mainActionsForm = form(screen),
mainActionsFormContainer = flexContainer(mainActionsForm),
selectButton = button(mainActionsFormContainer, { content: getString('home.selectHost'), hidden: hosts.length === 0 }),
addButton = button(mainActionsFormContainer, { content: getString('addHost') });
addButton.on('press', () => callback({ action: 'add-host' }));
@ -30,6 +30,13 @@ export default (
.on('press', () => callback({ action: 'delete-account' }));
button(mainActionsFormContainer, { content: getString('quit') })
.on('press', () => callback({ action: 'quit' }));
// Same reblessed Form Escape workaround for the main actions: registered after
// the Form's built-in Escape->self-focus handler (so it has the final word), it
// returns focus to the primary action instead of the non-actionable container.
mainActionsForm.on('element keypress', (element, chars, key) => {
if(key.name === 'escape')
(hosts.length ? selectButton : addButton).focus();
});
let selectedHost;
const
tableHeight = screen.height - 15,
@ -47,9 +54,16 @@ export default (
])
}
);
// The host list is built per view mount (the dashboard remounts this view on
// every navigation cycle), so leave the list with its own Escape binding
// instead of subscribing to the session-wide emitter, where stale closures
// would accumulate and keep stealing focus (issue #2).
hostsTable.rows.on('element keypress', (element, chars, key) => {
if(key.name === 'escape')
selectButton.focus();
});
selectButton.on('press', () => {
hostsTable.focus();
emitter.on('escape', () => selectButton.focus());
});
const
selectedHostForm = form(
@ -70,15 +84,25 @@ export default (
button(selectedHostActions, { content: getString('home.edit') })
.on('press', () => callback({ action: 'edit-host', host: selectedHost }));
const removeButton = button(selectedHostActions, { content: getString('home.remove') }, { color: 'yellow' });
cancelButton.on('press', () => {
const cancelSelectedHostForm = () => {
selectedHostForm.hide();
removeButton.setContent(getString('home.remove'));
screen.render();
hostsTable.focus();
removeButton.setContent(getString('home.remove'));
};
cancelButton.on('press', cancelSelectedHostForm);
// reblessed Forms steal Escape to focus their own, non-actionable container.
// This listener is registered after the Form's built-in one, so it has the final
// word and Escape actually cancels the dialog instead of stranding focus where
// the next Tab/Down would be consumed without moving (issue #2).
selectedHostForm.on('element keypress', (element, chars, key) => {
if(key.name === 'escape')
cancelSelectedHostForm();
});
hostsTable.rows.on('select', (undefined, index) => {
selectedHost = hosts[index];
selectedHostText.setContent(`${getString('home.selectedHost')} : ${selectedHost.name}`);
selectedHostForm.resetSelected();
selectedHostForm.show();
screen.render();
cancelButton.focus();

View file

@ -0,0 +1,245 @@
// Regression tests for KaKi87/sshception issue #2:
// "[TUI] Navigation requiring tab/arrow down twice when moving focus to next element"
//
// The TUI is a reblessed (https://github.com/kenan238/reblessed) screen driven by an
// SSH client PTY, so focus navigation is exercised the same way: a virtual terminal
// input stream is fed raw key sequences and the focused widget is asserted.
//
// Run with: node --test tests/
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { PassThrough } from 'node:stream';
import blessed from 'blessed';
import { createEmitter } from 'simpler-emitter';
import { home } from '../src/tui/views/main.js';
const hosts = [
{ name: 'host1', address: '192.0.2.10', port: 22, username: 'u1', password: 'p1' },
{ name: 'host2', address: '192.0.2.20', port: 22, username: 'u2', password: 'p2' }
];
const ENTER = '\r';
const TAB = '\t';
const SHIFT_TAB = '\x1b[Z';
const ESCAPE = '\x1b';
const DOWN = '\x1b[B';
const UP = '\x1b[A';
const boot = ({ hostList = hosts, emitter: providedEmitter } = {}) => {
const input = new PassThrough();
const output = new PassThrough();
output.on('data', () => {});
const screen = blessed.screen({
title: 'SSHception-test',
smartCSR: true,
terminal: 'xterm-256color',
input,
output,
autoPadding: true,
width: 100,
height: 30
});
// Mirror src/tui/main.js: Escape is forwarded globally to the emitter bus.
const emitter = providedEmitter || createEmitter();
screen.on('keypress', (undefined, { name }) => {
if(name === 'escape')
emitter.emit('escape');
});
home(screen)({
hosts: hostList,
isTotpEnabled: false,
emitter,
callback: () => {}
});
return {
screen,
emitter,
press: async sequence => {
input.write(sequence);
await new Promise(resolve => setTimeout(resolve, 25));
}
};
};
const textOf = element => (element?.content || '').toString().replace(/\s+/g, ' ').trim();
const containsButton = (element, label) => {
if(element.type === 'button' && textOf(element) === label)
return true;
return (element.children || []).some(child => containsButton(child, label));
};
const find = (screen, predicate) => {
let found;
const walk = element => {
if(!found && predicate(element))
found = element;
element.children?.forEach(walk);
};
screen.children.forEach(walk);
return found;
};
const findHostList = screen => find(screen, element => element.type === 'list');
const findSelectedHostForm = screen =>
find(screen, element => element.type === 'form' && containsButton(element, 'Cancel'));
const focusedLabel = screen => {
const element = screen.focused;
return element ? `${element.type}:${textOf(element).slice(0, 30)}` : '<none>';
};
test('selecting a host and pressing Escape cancels the dialog in one step', async () => {
const { screen, press } = boot();
await press(ENTER); // "Select host"
const hostList = findHostList(screen);
assert.equal(screen.focused, hostList, 'host list gets focus when selecting a host');
await press(ENTER); // select first host
const selectedHostForm = findSelectedHostForm(screen);
assert.equal(selectedHostForm.visible, true, 'selected-host actions are shown');
assert.equal(textOf(screen.focused), 'Cancel', 'Cancel is the focused action');
await press(ESCAPE);
assert.equal(selectedHostForm.visible, false, 'Escape hides the selected-host dialog');
assert.equal(screen.focused, hostList, 'Escape returns focus to the host list');
assert.equal(screen.focused.visible, true, 'nothing invisible is focused afterwards');
screen.destroy();
});
test('Tab and arrow move to an actionable element on every press, also after reopening', async () => {
const { screen, press } = boot();
const hostList = findHostList(screen);
await press(ENTER); // "Select host" -> host list
assert.equal(screen.focused, hostList, 'starts inside the host list');
for(let attempt = 0; attempt < 2; attempt++){
await press(ENTER); // select first host row -> Cancel
await press(TAB);
assert.equal(textOf(screen.focused), 'Connect', `Tab #1 (attempt ${attempt + 1})`);
await press(TAB);
assert.equal(textOf(screen.focused), 'Edit', `Tab #2 (attempt ${attempt + 1})`);
await press(TAB);
assert.equal(textOf(screen.focused), 'Remove', `Tab #3 (attempt ${attempt + 1})`);
await press(TAB);
assert.equal(textOf(screen.focused), 'Cancel', `Tab #4 wraps back (attempt ${attempt + 1})`);
await press(SHIFT_TAB);
assert.equal(textOf(screen.focused), 'Remove', 'Shift-Tab moves backwards');
await press(UP);
assert.equal(textOf(screen.focused), 'Edit', 'Up moves backwards');
await press(DOWN);
assert.equal(textOf(screen.focused), 'Remove', 'Down moves forwards');
await press(ESCAPE);
assert.equal(screen.focused, hostList, `Escape lands back on the list (attempt ${attempt + 1})`);
assert.notEqual(screen.focused.type, 'form', 'focus never rests on a form container');
}
screen.destroy();
});
test('Escape from main actions or the host list reaches an actionable element', async () => {
const { screen, press } = boot();
assert.equal(textOf(screen.focused), 'Select host');
await press(TAB);
assert.equal(textOf(screen.focused), 'Add host');
await press(ESCAPE);
assert.equal(textOf(screen.focused), 'Select host', 'escape from Add host selects the primary action');
await press(ENTER); // into host list
await press(ESCAPE);
assert.equal(textOf(screen.focused), 'Select host', 'escape from the host list restores Select host');
assert.equal(screen.focused.visible, true);
screen.destroy();
});
test('Escape bindings are mount-local: repeated home mounts add no global listeners', async () => {
// src/tui/controllers/dashboard.js remounts the home view (same screen, same
// session-wide emitter) every time the user returns from a sub-screen. The old
// code subscribed a new emitter listener on every "Select host" press, piling up
// stale closures; the view must not subscribe to the shared emitter at all.
const emitter = createEmitter();
let escapeSubscriptions = 0;
emitter.on = (event, listener) => {
if(event === 'escape')
escapeSubscriptions++;
return undefined;
};
const input = new PassThrough();
const output = new PassThrough();
output.on('data', () => {});
const screen = blessed.screen({
title: 'SSHception-test',
smartCSR: true,
terminal: 'xterm-256color',
input,
output,
autoPadding: true,
width: 100,
height: 30
});
screen.on('keypress', (undefined, { name }) => {
if(name === 'escape')
emitter.emit('escape');
});
for(let mount = 0; mount < 3; mount++)
home(screen)({ hosts, isTotpEnabled: false, emitter, callback: () => {} });
assert.equal(escapeSubscriptions, 0, `home must own no shared Emitter listeners, found ${escapeSubscriptions}`);
// Drive the live (latest) mount and check focus semantics, not widget identity:
// previous mounts remain stacked as stale, hidden siblings.
input.write(ENTER);
await new Promise(resolve => setTimeout(resolve, 25));
assert.equal(screen.focused.type, 'list', 'Select host opens the host list on the live mount');
assert.equal(screen.focused.visible, true);
input.write(ESCAPE);
await new Promise(resolve => setTimeout(resolve, 25));
assert.equal(textOf(screen.focused), 'Select host', 'Escape from the list is handled on the live mount');
assert.equal(screen.focused.visible, true);
screen.destroy();
});
test('with no hosts, traversal stays on visible actionable elements only', async () => {
const { screen, press } = boot({ hostList: [] });
const expected = [ 'Add host', 'Enable TOTP', 'Edit user/pass', 'Delete account', 'Quit', 'Add host' ];
assert.equal(screen.focused.visible, true);
for(let i = 0; i < expected.length; i++){
await press(DOWN);
assert.equal(screen.focused.visible, true, `Down #${i + 1} reaches an actionable element: ${focusedLabel(screen)}`);
assert.notEqual(screen.focused.type, 'form', 'focus never rests on the non-actionable form container');
assert.equal(textOf(screen.focused), expected[i], `navigation order step ${i + 1}`);
}
screen.destroy();
});
test('the hidden selected-host dialog is never focusable', async () => {
const { screen, press } = boot();
const selectedHostForm = findSelectedHostForm(screen);
for(let i = 0; i < 8; i++){
await press(TAB);
assert.notEqual(screen.focused, selectedHostForm, 'hidden dialog form skipped');
assert.notEqual(screen.focused.parent, selectedHostForm, 'hidden dialog buttons skipped');
}
screen.destroy();
});