Merge pull request #77 from foundersandcoders/refactor/file-picker-app-shell

JasonWarrenUK · web-flow · commit 5f23f60aed01 · 2026-07-06T15:25:28.000+01:00
diff --git a/docs/roadmaps/v5a-2606/enhanced.md b/docs/roadmaps/v5a-2606/enhanced.md
@@ -14,8 +14,8 @@ because the shell rollout and signature features build on them.
 | Phase | Focus                                                        | Status |
 |-------|--------------------------------------------------------------|--------|
 | **A** |        Foundations (theme, layout primitives, keymap)        | Complete |
-| **S** |         Security & dependency maintenance (Dependabot)       | In Progress (S1 done, S2 open) |
-| **B** |               App-shell rollout across screens               | Blocked (needs S2) |
+| **S** |         Security & dependency maintenance (Dependabot)       | Complete |
+| **B** |               App-shell rollout across screens               | In Progress (B1, B2 done) |
 | **C** | Signature UX features (help, toasts, progress, transitions)  | Ready |
 | **D** | Polish (palette, command palette, dark mode, schema display) | Blocked (needs B) |
 | **E** |       Tutorial & demo resources (Charm VHS recordings)       | Blocked (needs B/C) |
@@ -178,11 +178,42 @@ Not TUI redesign work, but tracked here since this is the active roadmap.
 One branch per screen cluster; each adopts the shell, bordered panels, the keymap
 registry, and a header breadcrumb.
 
-- [ ] **TR.B1** `refactor/dashboard-app-shell` — Dashboard onto shell + panels +
+- [x] **TR.B1** `refactor/dashboard-app-shell` — Dashboard onto shell + panels +
       keymap; add a **Recent Activity** panel sourced from submission history.
-      — **depends on TR.S2**
-- [ ] **TR.B2** `refactor/file-picker-app-shell` — File picker; framed list +
-      (later) preview panel; consistent nav keys. — **depends on TR.S2**
+      — **depends on TR.S2**. **Done:** rebuilt on `appShell()` + `panel()`
+      (menu panel + Recent Activity panel side by side), `Keymap` drives the
+      footer keybar (nav hint, number shortcuts, quit/back both resolve
+      `quit` at root). Recent Activity shows the 5 most recent submissions
+      from `createStorage().loadHistory()`, newest-first, with an
+      "Unknown date" fallback for unparseable timestamps. Merged via PR #76.
+- [x] **TR.B2** `refactor/file-picker-app-shell` — File picker; framed list +
+      (later) preview panel; consistent nav keys. — **depends on TR.S2**.
+      **Done:** replaced the hand-rolled root/header/footer/keypress-listener
+      with `appShell()` + `panel()` + `Keymap`, following the TR.B1 dashboard
+      as the reference adopter. Header breadcrumb shows the screen title
+      (e.g. "Select CSV File"); the file-list panel's **border title shows
+      the live current-directory path**, updating on both directory-entry
+      navigation and Backspace-up. `Keymap` bindings: nav hint (bar-only,
+      `SelectRenderable` owns arrows), Enter (`selectCurrent()`), Backspace
+      (extracted into `goUpDirectory()`); both ESC and `q` pop (file-picker
+      has no quit-to-desktop concept, unchanged from pre-refactor
+      behaviour). All caller data contracts preserved unchanged: the
+      `check-current`/`check-previous` two-step flow, `mapping-create`,
+      default single-file workflows, and the `selectionMode: 'directory'` +
+      `__select__` sentinel + `fieldKey` pop-back used by Settings. The
+      "(later) preview panel" is deferred — the body is a row `BoxRenderable`
+      wrapping the single file-list panel, so a second pane can be added
+      alongside it without restructuring. Added two tests
+      (`tests/tui/screens/file-picker.test.ts`) asserting the footer keybar
+      and the panel-title path display; all other existing tests updated to
+      match the new shell/keymap wiring where needed. Verified manually via
+      `bun run cli` in a real terminal (tmux-driven): Convert flow, Settings'
+      directory-mode picker (sentinel select + `fieldKey` round-trip),
+      directory navigation in/out with live title updates, ESC/backspace
+      behaviour. `bun run test:svelte` 53/53 files, 600/600 tests green;
+      `bun run test:core` 474/474 unaffected; no new `tsc` diagnostics (the
+      5 pre-existing `updateSelectOptions` possibly-undefined errors predate
+      this branch, confirmed via `git stash` comparison).
 - [ ] **TR.B3** `refactor/workflow-app-shell` — Processing screen into the frame
       (sets up TR.C3). — **depends on TR.S2**
 - [ ] **TR.B4** `refactor/results-screens-app-shell` — validation-explorer +
diff --git a/src/tui/screens/file-picker.ts b/src/tui/screens/file-picker.ts
@@ -8,12 +8,13 @@ import {
 	TextRenderable,
 	SelectRenderable,
 	SelectRenderableEvents,
-	type KeyEvent,
 	type SelectOption,
 } from '@opentui/core';
 import type { RenderContext, Renderer } from '../types';
-import { theme, PALETTE, symbols } from '../../../assets/brand/theme';
+import { theme, symbols } from '../../../assets/brand/theme';
 import type { Screen, ScreenResult, ScreenData } from '../utils/router';
+import { Keymap } from '../utils/keymap';
+import { appShell, panel, type Panel } from '../components';
 
 interface FileEntry {
 	name: string;
@@ -29,9 +30,9 @@ export class FilePicker implements Screen {
 	private currentPath: string;
 	private entries: FileEntry[] = [];
 	private select?: SelectRenderable;
-	private breadcrumb?: TextRenderable;
 	private emptyMessage?: TextRenderable;
-	private keyHandler?: (key: KeyEvent) => void;
+	private keymap?: Keymap;
+	private filePanel?: Panel;
 	private screenData?: ScreenData;
 
 	private title: string = 'Select CSV File';
@@ -69,42 +70,11 @@ export class FilePicker implements Screen {
 	}
 
 	cleanup(): void {
-		if (this.keyHandler) {
-			this.renderer.keyInput.off('keypress', this.keyHandler);
-		}
+		this.keymap?.detach(this.renderer);
 		this.renderer.root.remove(CONTAINER_ID);
 	}
 
 	private buildUI(resolve: (result: ScreenResult) => void): void {
-		// Root container
-		const container = new BoxRenderable(this.renderer, {
-			id: CONTAINER_ID,
-			flexDirection: 'column',
-			width: '100%',
-			height: '100%',
-			backgroundColor: theme.background,
-		});
-
-		// Header
-		const header = new BoxRenderable(this.renderer, {
-			flexDirection: 'column',
-		});
-
-		header.add(
-			new TextRenderable(this.renderer, {
-				content: this.title,
-				fg: theme.primary,
-			})
-		);
-
-		this.breadcrumb = new TextRenderable(this.renderer, {
-			content: this.shortenPath(this.currentPath),
-			fg: theme.textMuted,
-		});
-		header.add(this.breadcrumb);
-
-		container.add(header);
-
 		// Always create both renderables; toggle visibility based on whether there
 		// are entries. This avoids a one-way trap where starting in an empty directory
 		// means this.select is never created and updateSelectOptions() can never show
@@ -117,7 +87,6 @@ export class FilePicker implements Screen {
 			flexGrow: 1,
 			visible: !hasOptions,
 		});
-		container.add(this.emptyMessage);
 
 		this.select = new SelectRenderable(this.renderer, {
 			options: this.entriesToOptions(),
@@ -132,25 +101,63 @@ export class FilePicker implements Screen {
 			flexGrow: 1,
 			visible: hasOptions,
 		});
-		container.add(this.select);
-
-		// Status bar
-		container.add(
-			new TextRenderable(this.renderer, {
-				content: this.selectionMode === 'directory'
-					? '[↑↓] Nav  [ENTER] Open/Select  [BACKSPACE] Up  [ESC] Cancel'
-					: '[↑↓] Nav  [ENTER] Select  [BACKSPACE] Up Dir  [ESC] Back',
-				fg: theme.textMuted,
-			})
-		);
+
+		// Build keymap before the shell so its keybar can seed the footer
+		this.keymap = new Keymap({
+			onBack: () => resolve({ action: 'pop' }),
+			onQuit: () => resolve({ action: 'pop' }), // "q" also pops here — file-picker has no quit-to-desktop concept
+			bindings: [
+				// Nav hint — arrow keys handled by SelectRenderable; this is bar-only
+				{
+					keys: ['up', 'down', 'k', 'j'],
+					hint: `${symbols.arrows.up}${symbols.arrows.down}`,
+					label: 'Nav',
+					handler: () => {},
+				},
+				{
+					keys: ['enter'],
+					label: this.selectionMode === 'directory' ? 'Open/Select' : 'Select',
+					handler: () => this.select?.selectCurrent(),
+				},
+				{
+					keys: ['backspace'],
+					hint: 'BKSP',
+					label: 'Up',
+					handler: () => {
+						void this.goUpDirectory();
+					},
+				},
+			],
+		});
+
+		// Shell: header + breadcrumb, content region, footer keybar
+		const shell = appShell(this.renderer, {
+			id: CONTAINER_ID,
+			breadcrumb: this.title,
+			footer: this.keymap.toKeybar(),
+		});
+
+		// File-list panel — border title shows the current directory path
+		this.filePanel = panel(this.renderer, {
+			title: this.shortenPath(this.currentPath),
+			flexGrow: 1,
+		});
+		this.filePanel.add(this.emptyMessage);
+		this.filePanel.add(this.select);
+
+		// Body: the file-list panel (room for a preview pane alongside it later)
+		const body = new BoxRenderable(this.renderer, { flexDirection: 'row', flexGrow: 1 });
+		body.add(this.filePanel.box);
+		shell.content.add(body);
 
 		// Add to renderer
-		this.renderer.root.add(container);
+		this.renderer.root.add(shell.root);
 
 		// Focus and wire events — select is always created
 		if (hasOptions) {
 			this.select.focus();
 		}
+		this.keymap.attach(this.renderer);
 
 		this.select.on(
 			SelectRenderableEvents.ITEM_SELECTED,
@@ -173,9 +180,7 @@ export class FilePicker implements Screen {
 					this.currentPath = entry.path;
 					await this.loadDirectory();
 					this.updateSelectOptions();
-					if (this.breadcrumb) {
-						this.breadcrumb.content = this.shortenPath(this.currentPath);
-					}
+					this.filePanel?.setTitle(this.shortenPath(this.currentPath));
 				} else if (this.workflowType === 'check-current') {
 					// First step of check flow: selected current file, now pick previous
 					resolve({
@@ -224,24 +229,17 @@ export class FilePicker implements Screen {
 				}
 			}
 		);
+	}
 
-		// Screen-level key handler
-		this.keyHandler = async (key: KeyEvent) => {
-			if (key.name === 'backspace') {
-				const parent = path.dirname(this.currentPath);
-				if (parent !== this.currentPath) {
-					this.currentPath = parent;
-					await this.loadDirectory();
-					this.updateSelectOptions();
-					if (this.breadcrumb) {
-						this.breadcrumb.content = this.shortenPath(this.currentPath);
-					}
-				}
-			} else if (key.name === 'escape' || key.name === 'q') {
-				resolve({ action: 'pop' });
-			}
-		};
-		this.renderer.keyInput.on('keypress', this.keyHandler);
+	/** Navigate to the parent directory (bound to Backspace via the keymap). */
+	private async goUpDirectory(): Promise<void> {
+		const parent = path.dirname(this.currentPath);
+		if (parent === this.currentPath) return;
+
+		this.currentPath = parent;
+		await this.loadDirectory();
+		this.updateSelectOptions();
+		this.filePanel?.setTitle(this.shortenPath(this.currentPath));
 	}
 
 	private updateSelectOptions(): void {
diff --git a/tests/tui/screens/file-picker.test.ts b/tests/tui/screens/file-picker.test.ts
@@ -138,6 +138,44 @@ describe('FilePicker', () => {
 		expect(shortened).toBe('/tmp/data.csv');
 	});
 
+	it('renders a footer keybar containing "Nav" and "Select" driven by the keymap', async () => {
+		const screen = new FilePicker(mockContext);
+
+		(fs.readdir as any).mockResolvedValue(filePickerFixtures.mixedDirectory);
+
+		screen.render(); // Don't await
+
+		await new Promise((resolve) => setTimeout(resolve, 10));
+
+		// The shell root is the single child added to renderer.root
+		const shellRoot = (mockContext.renderer.root.add as any).mock.calls[0][0];
+		const children = shellRoot.getChildren();
+		// Last child is the footer TextRenderable (header, content, footer)
+		const footer = children[children.length - 1];
+		expect(footer.constructor.name).toBe('TextRenderable');
+		const footerText: string = footer.content.chunks[0].text;
+		expect(footerText).toContain('Nav');
+		expect(footerText).toContain('Select');
+
+		screen.cleanup();
+	});
+
+	it('shows the current directory path as the file-list panel title', async () => {
+		const screen = new FilePicker(mockContext);
+
+		(fs.readdir as any).mockResolvedValue(filePickerFixtures.mixedDirectory);
+
+		screen.render(); // Don't await
+
+		await new Promise((resolve) => setTimeout(resolve, 10));
+
+		const filePanel = (screen as any).filePanel;
+		expect(filePanel).toBeDefined();
+		expect(filePanel.box.title).toBe((screen as any).shortenPath(process.cwd()));
+
+		screen.cleanup();
+	});
+
 	it('shows empty message when no CSV files found', async () => {
 		const screen = new FilePicker(mockContext);
 
