Skip to content

Commit 836fe1d

Browse files
aesopovaesopov
committed
feat: enhance keyboard navigation in NestedPopoverMenu and update ExtensionsPanel with new filter menu
1 parent ea4fef6 commit 836fe1d

4 files changed

Lines changed: 375 additions & 49 deletions

File tree

‎docs/keyboard-interaction-architecture.md‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# Keyboard Interaction Architecture
2+
3+
## Overview
4+
5+
DotDir keyboard handling is built from three layers:
6+
7+
1. **DOM event routing**
8+
2. **Focus-aware command routing**
9+
3. **Command handlers**
10+
11+
The current design keeps one keyboard abstraction in the system: commands.
12+
13+
- **Surface navigation** uses shared generic commands like `cursorLeft` and `selectRight`
14+
- **Editing-specific behavior** uses dedicated commands like `commandLine.execute`
15+
16+
## Event Flow
17+
18+
The main entry point is [useCommandRouting.ts](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/features/commands/useCommandRouting.ts).
19+
20+
For most keys:
21+
22+
1. A `keydown` is captured on the app root.
23+
2. `CommandRegistry.handleKeyboardEvent(...)` resolves the keybinding to a command id.
24+
3. The most recently registered handler for that command runs.
25+
26+
There is also a window-level fallback for panel mode:
27+
28+
- plain `Tab`
29+
- function keys like `F5`, `F6`, `F10`, `F11`
30+
31+
That fallback exists because some special keys do not reliably travel through the normal focused-element path in the webview/browser environment.
32+
33+
## Focus Layers
34+
35+
Keyboard routing depends on the current logical focus layer, not just `document.activeElement`.
36+
37+
Focus layers are managed by [focusContext.ts](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/focusContext.ts) and exposed to the command system through `CommandRegistry.setFocusLayerGetter(...)`.
38+
39+
Current `when` clauses rely on focus predicates such as:
40+
41+
- `focusPanel`
42+
- `focusMenu`
43+
- `focusViewer`
44+
- `focusEditor`
45+
- `focusModal`
46+
47+
This is what lets the same physical key mean different things in different surfaces.
48+
49+
## Command Families
50+
51+
### Shared Navigation Commands
52+
53+
These commands are intentionally generic:
54+
55+
- `cursorUp`
56+
- `cursorDown`
57+
- `cursorLeft`
58+
- `cursorRight`
59+
- `cursorHome`
60+
- `cursorEnd`
61+
- `cursorPageUp`
62+
- `cursorPageDown`
63+
- `selectUp`
64+
- `selectDown`
65+
- `selectLeft`
66+
- `selectRight`
67+
- `selectHome`
68+
- `selectEnd`
69+
- `selectPageUp`
70+
- `selectPageDown`
71+
- `accept`
72+
- `cancel`
73+
74+
They are defined in [commandIds.ts](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/features/commands/commandIds.ts).
75+
76+
These commands are appropriate when the meaning is “move around the current interactive surface”.
77+
78+
That is why FileList uses the shared `cursor*` and `select*` commands directly now, instead of older aliases like `filelist.cursorLeft`.
79+
80+
### Command Line Editing Commands
81+
82+
The command line keeps separate command ids for behavior that is truly editing-specific, for example:
83+
84+
- `commandLine.cursorWordLeft`
85+
- `commandLine.selectWordRight`
86+
- `commandLine.deleteLeft`
87+
- `commandLine.execute`
88+
89+
These are registered in [CommandLine.tsx](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/features/command-line/CommandLine/CommandLine.tsx).
90+
91+
These are not just alternate names for `cursorLeft`.
92+
93+
They represent **text editing behavior**, not generic surface navigation.
94+
95+
## Command Ownership
96+
97+
[CommandRegistry](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/features/commands/commands.ts) now uses a simple **latest registration wins** rule.
98+
99+
That means:
100+
101+
- multiple parts of the app may still reuse the same command id
102+
- only one handler runs for a given command execution
103+
- ownership is expressed by registration lifecycle, not by registry predicates
104+
105+
So the important rule is:
106+
107+
- if a surface owns a shared command right now, it registers it
108+
- if it stops owning that command, it unregisters it
109+
110+
This is what allows shared navigation commands to be reused safely across panels, menus, command palette, autocomplete, and command line.
111+
112+
## Shared Commands vs Dedicated Commands
113+
114+
The command line now reuses shared navigation commands for:
115+
116+
- `cursorLeft`
117+
- `cursorRight`
118+
- `cursorHome`
119+
- `cursorEnd`
120+
- `selectLeft`
121+
- `selectRight`
122+
- `selectHome`
123+
- `selectEnd`
124+
125+
Those handlers are registered by [CommandLine.tsx](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/features/command-line/CommandLine/CommandLine.tsx) only while the command line owns editing navigation.
126+
127+
So the split is now:
128+
129+
- **FileList/menu aliases were redundant and removed**
130+
- **Command line uses shared commands for basic cursor/selection movement**
131+
- **Command line keeps dedicated commands for word movement, delete, clipboard, and execute/clear**
132+
133+
## Keybinding Layers
134+
135+
Keybindings are registered through [registerKeybindings.ts](/Users/mike/github/dotdirfm/dotdir/packages/ui/lib/features/commands/registerKeybindings.ts).
136+
137+
Resolution order is:
138+
139+
1. default
140+
2. extension
141+
3. user
142+
143+
Later layers override earlier ones.
144+
145+
This means a key like `Left` may resolve to:
146+
147+
- `cursorLeft` in panel/menu contexts
148+
- `cursorLeft` in command-line editing too, with the command line registering that command while it owns editing
149+
150+
The distinction happens through `when` clauses, focus layers, and registration ownership, not through a separate intent layer.
151+
152+
## Current Rules of Thumb
153+
154+
When adding keyboard behavior:
155+
156+
- Use shared `cursor*` / `select*` / `accept` / `cancel` commands for focus-surface navigation.
157+
- Use dedicated command ids for true domain-specific behavior, especially editing commands.
158+
- Prefer `when` clauses plus focus layers over duplicated routing layers.
159+
- Popup surfaces such as menus, command palette, and autocomplete should expose their own focus layer and command handlers instead of inventing a separate keyboard abstraction.
160+
- If a key is flaky in the webview, fix it in the routing layer rather than adding one-off listeners across feature components.
161+
162+
## Current Direction
163+
164+
The current architecture is:
165+
166+
- shared commands for navigation semantics
167+
- dedicated commands for buffer-editing semantics
168+
- lifecycle-based command ownership so surfaces reuse command ids safely
169+
- no separate interaction-intent layer between keybindings and commands

‎packages/ui/lib/components/NestedPopoverMenu/NestedPopoverMenu.tsx‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,13 +452,70 @@ export const NestedPopoverMenu = forwardRef<NestedPopoverMenuHandle, NestedPopov
452452
width: contentSize?.width,
453453
height: contentSize?.height,
454454
} as React.CSSProperties}
455+
onKeyDownCapture={(event) => {
456+
switch (event.key) {
457+
case "ArrowUp":
458+
event.preventDefault();
459+
event.stopPropagation();
460+
handleMenuCommand(CURSOR_UP);
461+
return;
462+
case "ArrowDown":
463+
event.preventDefault();
464+
event.stopPropagation();
465+
handleMenuCommand(CURSOR_DOWN);
466+
return;
467+
case "ArrowLeft":
468+
event.preventDefault();
469+
event.stopPropagation();
470+
handleMenuCommand(CURSOR_LEFT);
471+
return;
472+
case "ArrowRight":
473+
event.preventDefault();
474+
event.stopPropagation();
475+
handleMenuCommand(CURSOR_RIGHT);
476+
return;
477+
case "Home":
478+
event.preventDefault();
479+
event.stopPropagation();
480+
handleMenuCommand(CURSOR_HOME);
481+
return;
482+
case "End":
483+
event.preventDefault();
484+
event.stopPropagation();
485+
handleMenuCommand(CURSOR_END);
486+
return;
487+
case "PageUp":
488+
event.preventDefault();
489+
event.stopPropagation();
490+
handleMenuCommand(CURSOR_PAGE_UP);
491+
return;
492+
case "PageDown":
493+
event.preventDefault();
494+
event.stopPropagation();
495+
handleMenuCommand(CURSOR_PAGE_DOWN);
496+
return;
497+
case "Enter":
498+
event.preventDefault();
499+
event.stopPropagation();
500+
handleMenuCommand(ACCEPT);
501+
return;
502+
case "Escape":
503+
event.preventDefault();
504+
event.stopPropagation();
505+
handleMenuCommand(CANCEL);
506+
return;
507+
default:
508+
return;
509+
}
510+
}}
455511
>
456512
<div className={styles.viewport}>
457513
<div key={currentView.id} ref={observeCurrentContent} className={styles["screen-current"]}>
458514
<MenuViewBody
459515
view={currentView}
460516
canGoBack={stack.length > 1}
461517
onBack={popView}
518+
onCommand={handleMenuCommand}
462519
onItemClick={handleItemClick}
463520
selectedItemId={selectedItemId}
464521
setItemRef={(itemId, element) => {
@@ -478,6 +535,7 @@ function MenuViewBody({
478535
view,
479536
canGoBack,
480537
onBack,
538+
onCommand,
481539
onItemClick,
482540
selectedItemId,
483541
setItemRef,
@@ -487,6 +545,7 @@ function MenuViewBody({
487545
view: MenuView;
488546
canGoBack: boolean;
489547
onBack: () => void;
548+
onCommand: (commandId: string) => void;
490549
onItemClick: (item: NestedPopoverMenuItem) => void | Promise<void>;
491550
selectedItemId: string | null;
492551
setItemRef: (itemId: string, element: HTMLAnchorElement | null) => void;
@@ -548,6 +607,32 @@ function MenuViewBody({
548607
event.preventDefault();
549608
void onItemClick(item);
550609
}}
610+
onKeyDown={(event) => {
611+
switch (event.key) {
612+
case "ArrowLeft":
613+
event.preventDefault();
614+
event.stopPropagation();
615+
onCommand(CURSOR_LEFT);
616+
return;
617+
case "ArrowRight":
618+
event.preventDefault();
619+
event.stopPropagation();
620+
onCommand(CURSOR_RIGHT);
621+
return;
622+
case "Enter":
623+
event.preventDefault();
624+
event.stopPropagation();
625+
onCommand(ACCEPT);
626+
return;
627+
case "Escape":
628+
event.preventDefault();
629+
event.stopPropagation();
630+
onCommand(CANCEL);
631+
return;
632+
default:
633+
return;
634+
}
635+
}}
551636
onAuxClick={(event) => {
552637
if (event.button !== 1 || item.disabled || !item.onOpenInNewTab) return;
553638
event.preventDefault();

‎packages/ui/lib/features/extensions/ExtensionsPanel/ExtensionsPanel.module.css‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
align-items: center;
7878
justify-content: space-between;
7979
gap: 8px;
80-
border: 1px solid var(--border);
80+
border: 1px solid var(--input-border, var(--border));
8181
background: var(--bg);
8282
color: var(--fg);
8383
border-radius: 2px;
@@ -88,7 +88,11 @@
8888

8989
.ext-search-row input:focus,
9090
.ext-source-select:focus,
91-
.ext-filter-button:focus {
91+
.ext-source-select:focus-visible,
92+
.ext-filter-button:focus,
93+
.ext-filter-button:focus-visible {
94+
outline: 2px solid var(--border-active);
95+
outline-offset: 0;
9296
border-color: var(--border-active);
9397
}
9498

@@ -99,14 +103,23 @@
99103
}
100104

101105
.ext-filter-button {
106+
appearance: none;
107+
-webkit-appearance: none;
102108
min-width: 136px;
109+
background-image: none;
110+
box-shadow: none;
103111
cursor: pointer;
104112
}
105113

106114
.ext-filter-button.active {
107115
border-color: var(--border-active);
108116
}
109117

118+
.ext-filter-button:hover,
119+
.ext-filter-button:active {
120+
background: var(--bg);
121+
}
122+
110123
.ext-filter-menu {
111124
width: 220px;
112125
background: var(--bg);

0 commit comments

Comments
 (0)