🐛 Fix TUI focus navigation requiring Tab/arrow twice (#2) #14
3 changed files with 277 additions and 7 deletions
|
|
@ -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"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
245
tests/tui-home-focus.test.mjs
Normal file
245
tests/tui-home-focus.test.mjs
Normal 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();
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue