Skip to content

Commit 974a034

Browse files
authored
Keep host modifier chords out of pane commands and kill answers (#1278)
2 parents 9425c0a + 091650a commit 974a034

9 files changed

Lines changed: 107 additions & 9 deletions

‎docs/specs/layout.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ Source of truth: `handleDualTap` in `lib/src/components/wall/keyboard/handle-dua
218218

219219
`docs/specs/shortcuts.md` tables every binding; this section owns the dispatch behavior behind it.
220220

221+
**Must leave Cmd/Ctrl/Alt chords unhandled except swaps and printed symbols** ([Spatial navigation](#spatial-navigation)): Shift permits any binding; Option/AltGr permits `|`, `%`, `"`, `>`.
222+
221223
**Must use `,` to rename the selected terminal pane or Workspace tab in command mode**, without activating an inactive Workspace.
222224

223225
**Must support Workspace navigation in command mode through bare `1`–`9`, arrows, and `Enter`.** Digits select by strip position; out-of-range positions are consumed without switching. **Never bind `c`, `n`/`p`, `$`, or `&`.** Rename inputs and confirmation dialogs retain their own controls.
@@ -261,7 +263,7 @@ A split from an existing pane (`|`/`%`/`-`/`"` or the header split buttons) spaw
261263

262264
Dirty Tool close consent: `docs/specs/dor-tool.md` → Closing unsaved Tools.
263265

264-
`x`/`k` (or the kill button, which first leaves passthrough) shows a confirmation with a random lowercase letter; typing it confirms the kill. **Never draw `x`, `k`, or Reopen's `u` as that letter**, so neither a double-tap nor a reopen accepts it. Which closes skip it: `docs/specs/reopen.md` → "The rule"; under Labs one that would show it becomes a pending kill instead (`docs/specs/reopen.md` → "Labs: No-confirm delayed kill"). `Escape`, the cancel button, and clicking another panel cancel; any other key dismisses it.
266+
`x`/`k` (or the kill button, which first leaves passthrough) shows a confirmation with a random lowercase letter; typing it confirms the kill. **Never draw `x`, `k`, or Reopen's `u` as that letter**, so neither a double-tap nor a reopen accepts it. Which closes skip it: `docs/specs/reopen.md` → "The rule"; under Labs one that would show it becomes a pending kill instead (`docs/specs/reopen.md` → "Labs: No-confirm delayed kill"). `Escape`, the cancel button, and clicking another panel cancel; any other answer key dismisses it. **Never answer a kill prompt with a bare modifier or a Cmd/Ctrl/Alt chord.**
265267

266268
**Must return keyboard selection to the next surviving Door after killing a revealed Door**, falling back to previous Doors, then a pane only if no Doors remain. Apply this to confirmed and unasked kills only while the revealed pane, or the Door, is still selected in command mode; cancellation, refusal, or navigating away discards the return target.
267269

@@ -300,6 +302,8 @@ Source of truth: `resolvePaneElement` in `lib/src/components/wall/resolve-pane-e
300302

301303
## Spatial navigation
302304

305+
**Must leave Alt-modified arrows unhandled.**
306+
303307
**Must resolve arrow navigation through the engine-neutral `WallNav` seam to Lath's `neighborOf`, never a DOM rect scan.** Neighbor eligibility: `docs/specs/tiling-engine.md` → "Layout".
304308

305309
Back-navigation: a breadcrumb tracks the last navigation direction and origin pane, and **the opposite direction must return to the origin instead of doing a spatial lookup**, so asymmetric layouts navigate reversibly.

‎docs/specs/shortcuts.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ A focused cross-origin iframe surface swallows the gesture; the proxy shim detec
1616

1717
## Pane actions (command mode)
1818

19+
Modifier handling: [layout.md](layout.md#keyboard-shortcuts-command-mode).
20+
1921
| Key | Action | Description |
2022
|-----|--------|-------------|
2123
| `\|` or `%` | Split left/right | New pane to the right, selected, in passthrough. |
@@ -45,7 +47,7 @@ Standalone and the desktop playground — a bare Wall (VS Code) leaves these key
4547

4648
| Key | Action | Description |
4749
|-----|--------|-------------|
48-
| `↑` / `↓` / `←` / `→` | Move selection | Navigate panes and doors; opposite directions backtrack between panes. Down with no pane below selects the first door; Up from a door selects the last pane. Up from a top-edge pane highlights the active Workspace tab. |
50+
| `↑` / `↓` / `←` / `→` (without Cmd/Ctrl/Alt) | Move selection | Navigate panes and doors; opposite directions backtrack between panes. Down with no pane below selects the first door; Up from a door selects the last pane. Up from a top-edge pane highlights the active Workspace tab. |
4951
| `←` / `→` (Workspace strip) | Highlight tab / `+` | Traverse in strip order (`docs/specs/layout.md` → Workspace tabs), without activation, stopping at either end. Down returns to the originating live pane, or the first if gone. |
5052
| `⌘`+arrows or `Ctrl`+arrows | Swap surfaces | Swap the two panes' Surfaces; the opposite chord swaps back exactly. Either modifier, every platform; consumed no-op on non-pane chrome. |
5153

@@ -85,7 +87,7 @@ Every key not claimed above forwards to the embedded page while a screencast pan
8587
| `Shift+F10` / `ContextMenu` | Workspace alert settings | On a focused baseboard alarm button (`docs/specs/alert.md` → Settings dialog). |
8688
| `Shift+F10` / `ContextMenu` | Workspace tab menu | On a focused Workspace tab. |
8789
| `↑` / `↓`, `Home` / `End` | Move in a menu | Tab menu rows; `Enter` chooses. |
88-
| Prompted letter | Confirm kill | Type the letter shown to confirm; other keys reaching the prompt cancel (see layout's dispatch order). |
90+
| Prompted letter (unmodified, Shift optional) | Confirm kill | Type the letter shown; prompt routing: [layout.md](layout.md#kill-confirmation). |
8991

9092
## VS Code host
9193

‎lib/src/components/WorkspaceKillConfirm.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { useCallback, useEffect, useRef } from 'react';
22
import type { ModalLayer } from './design';
33
import { KillConfirmModal } from './KillConfirm';
4-
import { acceptsKillChar } from './wall/keyboard/handle-kill-confirm';
4+
import { acceptsKillChar, isKillAnswerKey } from './wall/keyboard/handle-kill-confirm';
55

66
/** One typed-letter interaction for Workspace close and move, window close, and
77
* app quit. */
@@ -28,7 +28,7 @@ export function WorkspaceKillConfirm({ char, onConfirm, onCancel, canConfirm, ti
2828
// ModalFrame handles Escape first on the same capture target.
2929
if (event.key === 'Escape' && event.defaultPrevented) return;
3030
// Cmd+Q is another quit request, never an answer even when the letter is q.
31-
if (event.metaKey || event.ctrlKey || event.altKey || ['Shift', 'Meta', 'Control', 'Alt'].includes(event.key)) return;
31+
if (!isKillAnswerKey(event)) return;
3232
event.preventDefault();
3333
event.stopImmediatePropagation();
3434
const latest = latestRef.current;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// @vitest-environment jsdom
2+
import { describe, expect, it, vi } from 'vitest';
3+
import { handleKillConfirm } from './handle-kill-confirm';
4+
import type { WallKeyboardCtx } from './types';
5+
6+
function context() {
7+
return { confirmKillRef: { current: { char: 'q' } }, acceptKill: vi.fn(), rejectKill: vi.fn() } as unknown as WallKeyboardCtx;
8+
}
9+
10+
describe('pane kill answers', () => {
11+
it.each([
12+
{ key: 'q', metaKey: true }, { key: 'q', ctrlKey: true }, { key: 'q', altKey: true },
13+
{ key: 'Meta' }, { key: 'Control' }, { key: 'Alt' }, { key: 'Shift' },
14+
])('leaves modifier/chord %j unanswered and available to the host', init => {
15+
const ctx = context();
16+
const event = new KeyboardEvent('keydown', { ...init, cancelable: true });
17+
expect(handleKillConfirm(event, ctx)).toBe(true);
18+
expect(ctx.acceptKill).not.toHaveBeenCalled();
19+
expect(ctx.rejectKill).not.toHaveBeenCalled();
20+
expect(event.defaultPrevented).toBe(false);
21+
});
22+
23+
it.each([{ key: 'q' }, { key: 'Q' }, { key: 'Q', shiftKey: true }])('accepts the typed letter %j', init => {
24+
const ctx = context();
25+
const event = new KeyboardEvent('keydown', { ...init, cancelable: true });
26+
expect(handleKillConfirm(event, ctx)).toBe(true);
27+
expect(ctx.acceptKill).toHaveBeenCalledOnce();
28+
expect(event.defaultPrevented).toBe(true);
29+
});
30+
31+
it('cancels on another ordinary key', () => {
32+
const ctx = context();
33+
handleKillConfirm(new KeyboardEvent('keydown', { key: 'Escape' }), ctx);
34+
expect(ctx.rejectKill).toHaveBeenCalledOnce();
35+
});
36+
});

‎lib/src/components/wall/keyboard/handle-kill-confirm.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { WallKeyboardCtx } from './types';
22

33
/**
44
* Whether a key accepts a staged kill confirmation: a case-insensitive match of
5-
* the confirm letter, so Caps Lock still confirms. Every other key rejects, which
5+
* the confirm letter, so Caps Lock still confirms. Every other answer key rejects, which
66
* is why the confirmation hijacks each key it sees rather than testing for one.
77
* Shared with `WorkspaceKillConfirm`, which listens outside every
88
* Wall (`docs/specs/shortcuts.md` → "Dialogs, menus & prompts").
@@ -11,13 +11,21 @@ export function acceptsKillChar(key: string, char: string): boolean {
1111
return key.toLowerCase() === char.toLowerCase();
1212
}
1313

14+
/** Modifiers and host chords never answer a typed-letter confirmation. Shift
15+
* may produce an uppercase letter, which acceptsKillChar handles normally. */
16+
export function isKillAnswerKey(event: KeyboardEvent): boolean {
17+
return !event.metaKey && !event.ctrlKey && !event.altKey
18+
&& !['Shift', 'Meta', 'Control', 'Alt'].includes(event.key);
19+
}
20+
1421
/**
1522
* Kill-confirmation second-key handler. Once a kill is staged in confirmKillRef,
16-
* we hijack every key: matching letter accepts, anything else rejects.
23+
* we hijack answer keys: matching letter accepts, anything else rejects.
1724
*/
1825
export function handleKillConfirm(e: KeyboardEvent, ctx: WallKeyboardCtx): boolean {
1926
const ck = ctx.confirmKillRef.current;
2027
if (!ck) return false;
28+
if (!isKillAnswerKey(e)) return true;
2129

2230
e.preventDefault();
2331
e.stopPropagation();

‎lib/src/components/wall/keyboard/handle-pane-navigation.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ function context(): WallKeyboardCtx {
2424
beforeEach(() => setWorkspaces({ workspaces: [{ id: 'ws-a', name: 'App' }, { id: 'ws-b', name: 'Build' }], activeId: 'ws-a' }));
2525

2626
describe('workspace row navigation', () => {
27+
it('leaves Alt+Arrow to its host instead of navigating the Wall', () => {
28+
const ctx = context();
29+
const event = new KeyboardEvent('keydown', { key: 'ArrowUp', altKey: true, cancelable: true });
30+
expect(handlePaneNavigation(event, ctx, { current: null })).toBe(false);
31+
expect(event.defaultPrevented).toBe(false);
32+
expect(ctx.selectWorkspace).not.toHaveBeenCalled();
33+
expect(ctx.nav.findInDirection).not.toHaveBeenCalled();
34+
});
35+
2736
it('moves from the top pane through tabs and + without activating, then returns to the pane', () => {
2837
const ctx = context();
2938
const history: NavHistoryRef = { current: null };

‎lib/src/components/wall/keyboard/handle-pane-navigation.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export function handlePaneNavigation(
1111
ctx: WallKeyboardCtx,
1212
navHistory: NavHistoryRef,
1313
): boolean {
14-
if (!isArrowKey(e.key) || e.metaKey || e.ctrlKey) {
14+
if (!isArrowKey(e.key) || e.metaKey || e.ctrlKey || e.altKey) {
1515
return false;
1616
}
1717
e.preventDefault();

‎lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,41 @@ function keydownMeta(key: string): KeyboardEvent {
7171
}
7272

7373
describe('handlePaneShortcuts kill behavior', () => {
74+
75+
it.each(['metaKey', 'ctrlKey', 'altKey'] as const)('leaves %s chords out of bare pane actions', modifier => {
76+
for (const key of ['x', 't', '-', 'Enter', ',']) {
77+
const ctx = makeCtx();
78+
const event = new KeyboardEvent('keydown', { key, [modifier]: true, cancelable: true });
79+
expect(handlePaneShortcuts(event, ctx, { current: null }), key).toBe(false);
80+
expect(event.defaultPrevented, key).toBe(false);
81+
expect(ctx.requestKill).not.toHaveBeenCalled();
82+
expect(ctx.enterTerminalMode).not.toHaveBeenCalled();
83+
expect(ctx.setRenamingPaneId).not.toHaveBeenCalled();
84+
expect(ctx.wallActionsRef.current.onSplitV).not.toHaveBeenCalled();
85+
}
86+
});
87+
88+
it.each([
89+
['|', { altKey: true }],
90+
['|', { ctrlKey: true, altKey: true }],
91+
['%', { altKey: true }],
92+
['"', { ctrlKey: true, altKey: true }],
93+
] as const)('accepts the printed split character %s through Option or AltGr', (key, modifiers) => {
94+
const ctx = makeCtx();
95+
const event = new KeyboardEvent('keydown', { key, ...modifiers, cancelable: true });
96+
expect(handlePaneShortcuts(event, ctx, { current: null })).toBe(true);
97+
expect(event.defaultPrevented).toBe(true);
98+
expect(key === '"' ? ctx.wallActionsRef.current.onSplitV : ctx.wallActionsRef.current.onSplitH)
99+
.toHaveBeenCalledWith('pane-a', 'keyboard');
100+
});
101+
102+
it('does not turn a Command chord into a printed symbol binding', () => {
103+
const ctx = makeCtx();
104+
const event = new KeyboardEvent('keydown', { key: '|', metaKey: true, altKey: true, cancelable: true });
105+
expect(handlePaneShortcuts(event, ctx, { current: null })).toBe(false);
106+
expect(event.defaultPrevented).toBe(false);
107+
});
108+
74109
beforeEach(() => vi.clearAllMocks());
75110

76111
it.each(['x', 'k'])('hands the %s gesture to the Wall kill request', (key) => {

‎lib/src/components/wall/keyboard/handle-pane-shortcuts.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,13 @@ export function handlePaneShortcuts(
2727
ctx: WallKeyboardCtx,
2828
navHistory: NavHistoryRef,
2929
): boolean {
30+
// Option and AltGr can print the symbol bindings on non-US keyboards.
31+
// Plain host chords still do not become pane actions; Cmd/Ctrl arrows are explicit.
32+
const printedSymbol = e.altKey && ['|', '%', '"', '>'].includes(e.key);
33+
if ((e.altKey && !printedSymbol) || ((e.metaKey || (e.ctrlKey && !printedSymbol)) && !isArrowKey(e.key))) return false;
3034
const sid = ctx.selectedIdRef.current;
3135
// Reopen acts on the Window, so it answers whatever is selected.
32-
if (e.key === 'u' && !e.metaKey && !e.ctrlKey && !e.altKey) {
36+
if (e.key === 'u') {
3337
e.preventDefault();
3438
e.stopPropagation();
3539
ctx.reopenClosed();

0 commit comments

Comments
 (0)