diff --git a/design.md b/design.md index 2faed0c..c2c0063 100644 --- a/design.md +++ b/design.md @@ -215,6 +215,8 @@ Owns the `Map`, exposes `tecode.workspace.openDocument/docu Layout state `{ sidebarVisible, sidebarWidth, panelVisible, panelHeight, activeView }` persists to `~/.config/tecode/state.json` on change (debounced) and on exit (*Req 6.4*). +**Initial editor focus** (*Req 6.7*): `EditorArea` gives its `EditorView`'s text plane keyboard focus the moment a document becomes the active tab — covering startup with a document already open, the first document opening on an empty workspace, and switching tabs — so typing works with no manual focus action. It skips this whenever the command palette, an input box, the find widget, or the explorer sidebar currently holds focus (read back through the shared context service, since none of those are `EditorArea`'s own descendants), so it never steals focus from something the user deliberately focused. + ### 8.3 EditorView (custom component, decision #2) Layers, back to front: diff --git a/packages/cli/src/editingHarness.tsx b/packages/cli/src/editingHarness.tsx index 7b52919..c35bedc 100644 --- a/packages/cli/src/editingHarness.tsx +++ b/packages/cli/src/editingHarness.tsx @@ -49,6 +49,7 @@ import { applyConfiguredTheme, ContextFocusTracker, getUserExtensionsDir, + ModalOverlay, Shell, ThemeProvider, type ContextService, @@ -309,6 +310,24 @@ function findAllFocusable(node: unknown): FocusableLike[] { * Returns `true` once a focused node is found that actually sets the key; * `false` (leaving every candidate focused-then-blurred) if none does — * e.g. no document is open yet, so `EditorArea` has no text plane to find. + * + * **Does NOT answer "who grants focus in the first place?"** (Issue #82's + * post-mortem): before the fix in `ui/shell.tsx`'s `EditorArea`, NOTHING in + * production ever imperatively focused the text plane on mount — this + * helper's own real-`.focus()`-walk masked that gap for over a year of + * tests, because every test that types goes through this function first, + * never through the production mount path alone. `EditorArea` now grants + * initial focus itself (on mount with a document already open, on a + * document opening later, and on switching tabs — see that component's own + * TSDoc for the exact rule and its do-not-steal guard), so a test that + * wants to prove typing works FROM PRODUCTION STARTUP ALONE must render the + * Shell and type WITHOUT calling this helper at all + * (`shell.initialFocus.test.tsx`'s "no focus assist" tests are exactly + * that) — calling `focusEditorText` before typing is still correct and + * still the right tool for every test that has a different, unrelated + * thing to prove (multi-cursor, undo/redo, syntax highlighting, …), it + * just no longer stands in for "does startup focus the editor" the way its + * mere existence previously, silently did. */ export function focusEditorText(rendererRoot: unknown, context: Pick): boolean { for (const node of findAllFocusable(rendererRoot)) { @@ -339,16 +358,25 @@ export type EditingShellDeps = Pick< | "editorSession" | "findService" | "highlightService" + | "modalService" >; /** - * Mount ` - * ` (design.md §8.1's component tree) onto OpenTUI's - * headless test renderer — the exact same tree - * `renderShell.tsx`'s `renderShellToTerminal` mounts onto a real terminal, - * just onto `@opentui/react/test-utils`'s `testRender` instead of a real - * `CliRenderer` (`shell.snapshot.test.tsx`'s top-of-file TSDoc documents why this is - * a full, real cell-grid renderer, not a fallback). + * Mount ` + * ` (design.md §8.1's component tree, + * `modalOverlay.tsx`'s "Mount point") onto OpenTUI's headless test + * renderer — the exact same tree `renderShell.tsx`'s `renderShellToTerminal` + * mounts onto a real terminal, just onto `@opentui/react/test-utils`'s + * `testRender` instead of a real `CliRenderer` (`shell.snapshot.test.tsx`'s + * top-of-file TSDoc documents why this is a full, real cell-grid renderer, + * not a fallback). `ModalOverlay` is mounted unconditionally (matching + * `renderShellToTerminal`'s own always-there `modalService` in production + * `main.ts`), so a test can drive `root.modalService.openQuickPick(...)`/ + * `openInputBox(...)` and observe the SAME `quickPickFocus`/`inputBoxFocus` + * context transitions production reports — Issue #82's "do not steal focus + * from the palette" regression is a real ordering interaction between + * `ModalOverlay` and `Shell`'s `EditorArea` that a harness omitting + * `ModalOverlay` could never exercise. */ export function renderEditingShell( deps: EditingShellDeps, @@ -367,6 +395,7 @@ export function renderEditingShell( findService={deps.findService} highlightService={deps.highlightService} /> + ); diff --git a/packages/core/src/ui/focus.tsx b/packages/core/src/ui/focus.tsx index 1a2c523..587a4ba 100644 --- a/packages/core/src/ui/focus.tsx +++ b/packages/core/src/ui/focus.tsx @@ -66,6 +66,44 @@ export function ContextFocusTracker(props: ContextFocusTrackerProps): ReactNode ); } +/** + * Returns the {@link ContextService} the nearest {@link ContextFocusTracker} + * provides, narrowed to `get`/`onDidChange` (Req 4.6) — for a component + * that needs to READ another region's focus-tracked context key directly + * (rather than only report its OWN focus transitions via + * {@link useFocusTracking}). `undefined` outside a + * {@link ContextFocusTracker} — matches this module's "no-op rather than + * throw when unwrapped" discipline for {@link useFocusTracking} itself, and + * a caller reads it the same way: an `undefined` context conservatively + * means "nothing is known to be holding focus" (`ui/shell.tsx`'s + * `EditorArea` initial-focus guard, Issue #82, is the first consumer — it + * must not steal focus from the command palette (`quickPickFocus`), an + * input box (`inputBoxFocus`), the find widget (`findWidgetFocus`), or the + * explorer (`explorerFocus`), none of which are `EditorArea`'s own React + * descendants: `ModalOverlay` is `Shell`'s sibling, `Sidebar` is `Shell`'s + * child — so the only way to see those keys from inside `EditorArea` is + * back through this shared `ContextService`, not through the component + * tree). + * + * **`onDidChange`** (CodeRabbit PR #83 follow-up on Issue #82's fix): + * `ContextService.onDidChange` is otherwise host-internal — "consumed by + * focus tracking and the keymap service, not extensions" (`api/create.ts`'s + * TSDoc on why `tecode.context` never exposes it). Exposing it here, to a + * component that already reads focus-tracked keys through this same hook, + * stays within that boundary (still core-internal, still nothing an + * extension can reach through `tecode.context`) while giving + * `EditorArea`'s do-not-steal guard a way to be told when a guard it + * deferred on has since cleared — a change to `quickPickFocus`/ + * `inputBoxFocus`/`findWidgetFocus`/`explorerFocus` is otherwise invisible + * to `EditorArea`'s own re-render cycle, since none of those keys are its + * own props and this hook always returns the SAME `ContextService` + * instance (no new value, hence no dependency-array-triggered re-run, ever + * comes from `focusContext` itself changing). + */ +export function useFocusContextService(): Pick | undefined { + return useContext(FocusContextServiceContext); +} + /** * Returns a `ref` callback that reports `key`'s value (`true`/`false`) to * the {@link ContextFocusTracker}-provided context service whenever the diff --git a/packages/core/src/ui/shell.initialFocus.test.tsx b/packages/core/src/ui/shell.initialFocus.test.tsx new file mode 100644 index 0000000..da3cfbf --- /dev/null +++ b/packages/core/src/ui/shell.initialFocus.test.tsx @@ -0,0 +1,425 @@ +/** + * Regression coverage for Issue #82 ("cannot type at all in a real + * terminal, but `ctrl+g` still works"): before this fix, NOTHING in the + * production component tree ever imperatively focused the editor's text + * plane — `editorTextFocus` stayed `undefined` forever after mount, so + * `editor/inputRouter.ts`'s `routeKeyEvent` gate (`if (!context. + * get("editorTextFocus")) return false;`) silently dropped every printable + * keystroke. The fix lives entirely in `shell.tsx`'s `EditorArea` (see its + * own TSDoc's "Initial/re-focus of the text plane" section for the exact + * rule and its do-not-steal guard) and `focus.tsx`'s new + * `useFocusContextService`. + * + * **No focus assist, anywhere, in any test below**: every test in this file + * mounts EXACTLY design.md §8.1's production tree (` + * `, + * `` added as `Shell`'s sibling where a test needs the + * palette, matching `renderShell.tsx`'s `renderShellToTerminal` exactly) + * and never calls `.focus()` on any node itself, never walks the render + * tree looking for a focusable candidate to focus (the `editingHarness.tsx` + * `focusEditorText`/this file's neighbor `shell.snapshot.test.tsx`'s + * "Finding 5" test idiom — both legitimate for THEIR OWN, different + * purposes, but exactly the shortcut that would prove nothing here), and + * never shortcuts `context.set("editorTextFocus", true)` directly. The only + * two things ever allowed to move real OpenTUI focus in this file are + * `EditorArea`'s own new effect (under test) and `ModalOverlay`/ + * `QuickPickBody`'s own real mount-focus effect (exercised as a genuine + * competing, real-world focus claim, not a fake). + * + * Uses the same real-collaborator, hand-rolled-fake harness as + * `shell.snapshot.test.tsx` (`createDocumentManager`, `createEditorSessionService`, + * `createContextService`, `createEditorInputRouter`, `createModalService` — + * every one a real `@tecode/core` factory, never a mock library), rather + * than the heavier `packages/cli` `AssemblyRoot`/extension-host harness: + * this bug and its fix live entirely inside `packages/core`'s `ui/`+ + * `editor/` wiring, so this is the narrowest tree that still reproduces the + * exact production composition and the exact real gate the bug lived in. + */ + +import { describe, expect, test } from "bun:test"; +import { act } from "react"; +import { TabSelectRenderable } from "@opentui/core"; +import { testRender } from "@opentui/react/test-utils"; +import { createDocumentManager, type DocumentManagerFs } from "../buffer/documentManager"; +import { pathToUri } from "../buffer/uri"; +import { createEditorInputRouter } from "../editor/inputRouter"; +import { createHostLog } from "../host/errors"; +import { createContextService } from "../keymap/context"; +import { createEditorSessionService } from "./editorSession"; +import { ContextFocusTracker } from "./focus"; +import { createLayoutStateService, type LayoutStateFs } from "./layoutState"; +import { createModalService } from "./modalService"; +import { ModalOverlay } from "./modalOverlay"; +import { createSlotRegistry } from "./slotRegistry"; +import { Shell } from "./shell"; +import { ThemeProvider } from "./theme"; + +/** A key event shaped exactly like `keyRouting.test.ts`'s/`shell.snapshot. + * test.tsx`'s own literal `KeyEventLike` object for a plain printable + * character — duplicated locally rather than imported (`editingHarness. + * tsx`'s own `keyOf`, kept as a separate copy for the same "not importing + * from a test file" reason its own TSDoc gives). */ +function printableKey(char: string) { + return { name: char, sequence: char, ctrl: false, shift: false, option: false, meta: false }; +} + +/** A `HostLog` sink that discards everything (matches `shell.snapshot. + * test.tsx`'s own identical helper) — these tests assert on focus/context + * behavior, not on what gets logged. */ +function createRecordingSink() { + return { error() {} }; +} + +/** An in-memory {@link LayoutStateFs} that starts with no `state.json` + * (matches `shell.snapshot.test.tsx`'s own identical helper). */ +function createEmptyLayoutFs(): LayoutStateFs { + return { + async readFile() { + throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + }, + async mkdir() {}, + async writeFile() {}, + }; +} + +/** The `slotRegistry`/`layoutState`/`context` trio every test below + * mounts `` against (matches `shell.snapshot.test.tsx`'s own + * identical helper) — `documents`/`editorSession`/`modalService` are each + * test's own concern, built separately per scenario. */ +function createHarness() { + const log = createHostLog(); + const sink = createRecordingSink(); + const slotRegistry = createSlotRegistry({ log }); + const layoutState = createLayoutStateService({ log, sink, path: "/state.json", fs: createEmptyLayoutFs() }); + const context = createContextService(); + return { slotRegistry, layoutState, context }; +} + +function createInMemoryFs(files: Record): DocumentManagerFs { + return { + async stat(path: string) { + if (!(path in files)) throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + return { size: files[path]!.length, mode: 0o644 }; + }, + async readFile(path: string) { + if (!(path in files)) throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + return files[path]!; + }, + async writeFile() {}, + async chmod() {}, + async rename() {}, + async unlink() {}, + }; +} + +/** Depth-first search for the rendered `` renderable (matches + * `shell.snapshot.test.tsx`'s own identical helper of the same name). */ +function findTabSelect(node: unknown): TabSelectRenderable | undefined { + if (node instanceof TabSelectRenderable) return node; + const candidate = node as { getChildren?: () => unknown[] }; + for (const child of candidate?.getChildren?.() ?? []) { + const found = findTabSelect(child); + if (found) return found; + } + return undefined; +} + +describe("Shell — initial editor focus (Req 4.6, 6.7, design.md §8.1; Issue #82)", () => { + test("mounting with a document already open focuses the text plane with no manual assist, so a printable key inserts into the document", async () => { + const { slotRegistry, layoutState, context } = createHarness(); + await layoutState.ready; + const documents = createDocumentManager({ + log: createHostLog(), + sink: createRecordingSink(), + fs: createInMemoryFs({ "/workspace/hello.ts": "hello" }), + }); + const editorSession = createEditorSessionService({ documents }); + // Open BEFORE mounting — Issue #82's most basic case: tecode starts on + // a workspace that already has a file open (this component's TSDoc's + // case 1). + const document = await documents.openDocument(pathToUri("/workspace/hello.ts")); + + const { renderOnce } = await testRender( + + + + + , + { width: 80, height: 20 }, + ); + await act(async () => { + await renderOnce(); + }); + + // The real gate `editor/inputRouter.ts`'s `routeKeyEvent` checks first + // — this is Issue #82's report made precise: with NOTHING having + // focused anything by hand, this must already be `true`. + expect(context.get("editorTextFocus")).toBe(true); + + const router = createEditorInputRouter({ context, editorSession }); + const handled = router.routeKeyEvent(printableKey("X")); + expect(handled).toBe(true); + expect(document.getLine(0)).toBe("Xhello"); + }); + + test("opening the first document after an empty-workspace startup focuses its text plane with no manual assist", async () => { + const { slotRegistry, layoutState, context } = createHarness(); + await layoutState.ready; + // The file exists on the (fake) filesystem from the start, but nothing + // opens it yet — an "empty workspace at launch" startup, exactly this + // component's TSDoc's case 2. + const documents = createDocumentManager({ + log: createHostLog(), + sink: createRecordingSink(), + fs: createInMemoryFs({ "/workspace/late.ts": "late" }), + }); + const editorSession = createEditorSessionService({ documents }); + + const { renderOnce } = await testRender( + + + + + , + { width: 80, height: 20 }, + ); + await act(async () => { + await renderOnce(); + }); + + // No document open yet: nothing for the initial-focus effect to grab, + // and nothing else claims the key either (this component's TSDoc's + // case 2's precondition). + expect(context.get("editorTextFocus")).toBeFalsy(); + + // The document opens LATER, through the SAME `documents`/ + // `editorSession` the Shell is already wired to — "a document opened + // later", not a fresh, disconnected `DocumentManager`. + const document = await documents.openDocument(pathToUri("/workspace/late.ts")); + await act(async () => { + await renderOnce(); + }); + + expect(context.get("editorTextFocus")).toBe(true); + const router = createEditorInputRouter({ context, editorSession }); + const handled = router.routeKeyEvent(printableKey("Y")); + expect(handled).toBe(true); + expect(document.getLine(0)).toBe("Ylate"); + }); + + test("does not steal focus from an open command palette when a document opens while it's showing", async () => { + const { slotRegistry, layoutState, context } = createHarness(); + await layoutState.ready; + const documents = createDocumentManager({ + log: createHostLog(), + sink: createRecordingSink(), + fs: createInMemoryFs({ "/workspace/hello.ts": "hello" }), + }); + const editorSession = createEditorSessionService({ documents }); + const modalService = createModalService(); + + // Mounts `` AND `` as siblings inside the same + // `` — exactly `renderShell.tsx`'s + // `renderShellToTerminal` composition (`modalOverlay.tsx`'s "Mount + // point" TSDoc), not a Shell-only tree — this test's whole point is the + // REAL interaction between the two. + const { renderOnce } = await testRender( + + + + + + , + { width: 80, height: 20 }, + ); + await act(async () => { + await renderOnce(); + }); + expect(context.get("editorTextFocus")).toBeFalsy(); + + // Open the real command palette — `ModalOverlay`'s own `QuickPickBody` + // mount effect calls a REAL `.focus()` on its filter input, exactly as + // `workbench.action.showCommands` does in production. + act(() => { + void modalService.openQuickPick([{ label: "Test Command" }]); + }); + await act(async () => { + await renderOnce(); + }); + expect(context.get("quickPickFocus")).toBe(true); + + // A document opens WHILE the palette is showing (e.g. an extension's + // own startup activation, or restoring a previous session's editor) — + // this component's TSDoc's case 2, now racing an already-open palette. + // Without the do-not-steal guard, the initial-focus effect would call + // `.focus()` on the new tab's text plane here, and OpenTUI's single + // global focus pointer would blur the palette's filter input out from + // under the user. + const document = await documents.openDocument(pathToUri("/workspace/hello.ts")); + await act(async () => { + await renderOnce(); + }); + + expect(context.get("quickPickFocus")).toBe(true); + expect(context.get("editorTextFocus")).toBeFalsy(); + + const router = createEditorInputRouter({ context, editorSession }); + const handled = router.routeKeyEvent(printableKey("X")); + // Dropped by the real `editorTextFocus` gate — never reaches the + // buffer "behind" the palette. + expect(handled).toBe(false); + expect(document.getLine(0)).toBe("hello"); + }); + + test("closing the palette after a document opened while it was showing retries the deferred focus (CodeRabbit PR #83 follow-up)", async () => { + // Continues exactly where "does not steal focus..." above stops: this + // is quick-open's real shape (Issue #82's most common path) — empty + // workspace, `ctrl+g`-equivalent opens the palette, a file is picked + // (which in production opens the document WHILE the palette is still + // showing, then the palette closes) — and proves the deferred focus + // attempt this scenario ARMS is not silently discarded once the guard + // clears, only DEFERRED. `ModalOverlay` restores focus only to + // whatever held it before the palette opened (`modalOverlay.tsx`'s + // `previousFocusRef`) — nothing did, here — so it cannot be what + // re-focuses the text plane; only `EditorArea`'s own retry can. + const { slotRegistry, layoutState, context } = createHarness(); + await layoutState.ready; + const documents = createDocumentManager({ + log: createHostLog(), + sink: createRecordingSink(), + fs: createInMemoryFs({ "/workspace/hello.ts": "hello" }), + }); + const editorSession = createEditorSessionService({ documents }); + const modalService = createModalService(); + + const { renderOnce } = await testRender( + + + + + + , + { width: 80, height: 20 }, + ); + await act(async () => { + await renderOnce(); + }); + + act(() => { + void modalService.openQuickPick([{ label: "Test Command" }]); + }); + await act(async () => { + await renderOnce(); + }); + expect(context.get("quickPickFocus")).toBe(true); + + // The document opens WHILE the palette is still showing — the + // do-not-steal guard defers the focus attempt (asserted already by the + // test above); this test continues past that point. + const document = await documents.openDocument(pathToUri("/workspace/hello.ts")); + await act(async () => { + await renderOnce(); + }); + expect(context.get("editorTextFocus")).toBeFalsy(); + + // The palette closes — `quickPickFocus` flips false. Nothing about + // `props.activeDocument?.uri` changes on this render (the active + // document is still "hello.ts"), so ONLY a retry driven by the context + // service's own `onDidChange` (not a uri-keyed effect re-run) can pick + // this back up. + act(() => { + modalService.cancel(); + }); + await act(async () => { + await renderOnce(); + }); + + expect(context.get("quickPickFocus")).toBe(false); + expect(context.get("editorTextFocus")).toBe(true); + + const router = createEditorInputRouter({ context, editorSession }); + const handled = router.routeKeyEvent(printableKey("Q")); + expect(handled).toBe(true); + expect(document.getLine(0)).toBe("Qhello"); + }); + + test("switching tabs re-focuses the newly active tab's text plane, with no manual assist", async () => { + const { slotRegistry, layoutState, context } = createHarness(); + await layoutState.ready; + const documents = createDocumentManager({ + log: createHostLog(), + sink: createRecordingSink(), + fs: createInMemoryFs({ "/workspace/a.ts": "AAAA", "/workspace/b.ts": "BBBB" }), + }); + const editorSession = createEditorSessionService({ documents }); + + const { renderOnce, renderer } = await testRender( + + + + + , + { width: 80, height: 20 }, + ); + await documents.openDocument(pathToUri("/workspace/a.ts")); + const docB = await documents.openDocument(pathToUri("/workspace/b.ts")); + await act(async () => { + await renderOnce(); + }); + // Startup focus already landed on tab A (this file's first test covers + // that case on its own) — asserted here only as this test's own + // precondition. + expect(context.get("editorTextFocus")).toBe(true); + + // Drive the REAL `` renderable exactly as `shell.snapshot. + // test.tsx`'s "selecting the second tab switches the active document's + // content" test does — not a direct `editorSession. + // setActiveDocumentUri(...)` call, which would bypass the very + // `EditorView` remount (`key={activeDocument.uri}`) this fix's "tab + // switch" case depends on. + const tabSelect = findTabSelect(renderer.root); + expect(tabSelect).toBeDefined(); + act(() => { + tabSelect?.moveRight(); + tabSelect?.selectCurrent(); + }); + await act(async () => { + await renderOnce(); + }); + + // Without this fix's "tab switch" case, `editorTextFocus` would be + // stuck `false` here: `focus.tsx`'s "detaching a still-focused node" + // fix force-blurs it the instant the OLD tab's `EditorView` unmounts, + // and nothing else would ever re-focus the NEW tab's text plane. + expect(context.get("editorTextFocus")).toBe(true); + const router = createEditorInputRouter({ context, editorSession }); + const handled = router.routeKeyEvent(printableKey("Z")); + expect(handled).toBe(true); + expect(docB.getLine(0)).toBe("ZBBBB"); + }); +}); diff --git a/packages/core/src/ui/shell.tsx b/packages/core/src/ui/shell.tsx index fddd835..d124b70 100644 --- a/packages/core/src/ui/shell.tsx +++ b/packages/core/src/ui/shell.tsx @@ -63,8 +63,9 @@ import type { FindService } from "./findService"; import type { HighlightService } from "../languages/highlightService"; import { FindWidget } from "./findWidget"; import type { FocusableNode } from "./focus"; -import { useFocusTracking } from "./focus"; +import { useFocusContextService, useFocusTracking } from "./focus"; import type { LayoutState, LayoutStateService } from "./layoutState"; +import { INPUT_BOX_FOCUS_CONTEXT_KEY, QUICK_PICK_FOCUS_CONTEXT_KEY } from "./modalCommands"; import type { SidebarPair, SlotRegistry, SlotViewEntry } from "./slotRegistry"; import { toColorInput, useTheme } from "./theme"; @@ -365,6 +366,82 @@ export interface EditorAreaProps { * level-triggered one — this must fire ONCE per close, not on every render * where find happens to already be closed, which would otherwise fight * the user clicking anywhere else in the shell while find stays shut). + * + * **Initial/re-focus of the text plane** (Req 4.6, 6.7; Issue #82): nothing + * ELSE in the app ever imperatively focuses the text plane on its own — + * before this fix, `editorTextFocus` simply stayed `undefined` forever + * after mount, so `editorInputRouter.routeKeyEvent`'s `if (!context. + * get("editorTextFocus")) return false;` gate (`editor/inputRouter.ts`) + * dropped every printable keystroke while chord-consumed commands with no + * `when` clause kept working — exactly the reported "can't type, but + * ctrl+g still works" symptom. A second edge-triggered `useEffect`, keyed + * on `props.activeDocument?.uri` (via `previousActiveUriRef` below), grants + * focus on exactly one transition — "the active document's uri just + * changed" — which covers all three startup/lifecycle cases in one rule: + * + * 1. **A document is already open at startup**: `previousActiveUriRef` + * starts at `undefined`, so the very first effect run (where + * `activeDocument` is already set) counts as a transition too — + * `EditorView`'s ref attaches during the SAME commit, before any + * effect runs, so `textPlaneNodeRef.current` is already valid. + * 2. **No document is open at startup, one opens later**: `uri` stays + * `undefined` (this effect no-ops — there is no text plane yet) until + * the document opens, at which point `uri` flips from `undefined` to + * a real value — the same transition as case 1. + * 3. **Switching tabs**: `uri` flips from one open document's uri to + * another's. `EditorView` remounts (`key={props.activeDocument.uri}` + * below), so by the time this effect runs `textPlaneNodeRef.current` + * already points at the NEW tab's node. Refocusing here matches "typing + * resumes immediately after switching tabs", the same way reopening a + * file does in case 2. + * + * Deliberately does NOT refire on a re-render that leaves `uri` unchanged + * — the same reason the find-close effect above is edge- rather than + * level-triggered: refiring on every unrelated re-render would fight a + * user who has since moved focus elsewhere on their own (the sidebar, the + * palette) every single time `EditorArea` re-renders for any reason. + * + * **Do-not-steal guard**: skipped entirely while the command palette + * (`quickPickFocus`), an input box (`inputBoxFocus`), the find widget + * (`findWidgetFocus`, or this tab's own `find.isOpen`), or the explorer + * sidebar (`explorerFocus`) legitimately holds focus. None of those are + * `EditorArea`'s own React descendants — `ModalOverlay` is `Shell`'s + * sibling (`modalOverlay.tsx`), `Sidebar` is `Shell`'s child — so this + * reads them back through the shared `ContextService` + * (`focus.tsx`'s `useFocusContextService`) rather than through React's own + * tree structure. Getting this wrong (focusing unconditionally) would + * steal focus out from under someone mid-typing in the palette the moment + * a document happens to open or a tab happens to switch underneath it — + * worse than the bug this effect exists to fix (this task's own framing). + * + * **A deferred attempt is retried, never discarded** (CodeRabbit PR #83 + * follow-up — Issue #82's own most common path: an empty workspace, the + * command palette opens the picked file, which activates it as the new + * tab WHILE the palette is still showing): the FIRST version of this + * effect advanced `previousActiveUriRef` unconditionally before checking + * the guard, so a transition that arrived while guarded was marked + * "already handled" and then discarded — nothing re-attempted it once the + * palette closed, because `focusContext` never changes identity (its + * `quickPickFocus` value living inside the `ContextService`'s internal Map + * is invisible to a React dependency array) and `props.activeDocument?.uri` + * does not change again on its own. `ModalOverlay` cannot rescue this + * either — it restores focus only to whatever was focused BEFORE the modal + * opened (`modalOverlay.tsx`'s `previousFocusRef`), which in this flow is + * not the new document's text plane at all. Fixed by separating two + * concerns that used to live in one ref: `previousActiveUriRef` ONLY + * detects "is this a genuinely new active-document uri" (advanced the + * instant a transition is seen, guard or no guard — this part was never + * the bug); `pendingFocusUriRef` holds whatever uri is still OWED a focus + * attempt, cleared only once `attemptFocus` actually succeeds. A second + * `useEffect` below subscribes to the context service's own `onDidChange` + * (exposed for exactly this by `focus.tsx`'s `useFocusContextService`) and + * calls `attemptFocus` again on every firing — cheap and safe to over-call, + * since `attemptFocus` itself no-ops the instant nothing is pending. + * Subscribing to `onDidChange` unfiltered (not just for the specific key + * that unblocked things) is deliberate: it means a deferred attempt is + * retried no matter WHICH of the four guards clears first — the command + * palette, an input box, the find widget, or the explorer sidebar — with + * no separate per-key wiring needed for each. */ export function EditorArea(props: EditorAreaProps): ReactNode { const theme = useTheme(); @@ -381,6 +458,80 @@ export function EditorArea(props: EditorAreaProps): ReactNode { } wasFindOpenRef.current = isFindOpen; }, [isFindOpen]); + + // Initial/re-focus of the text plane (Req 4.6, 6.7; Issue #82) — see this + // component's own TSDoc above ("Initial/re-focus of the text plane" and + // "A deferred attempt is retried, never discarded") for the full "why". + // `focusContext` is `undefined` outside a `ContextFocusTracker` (`focus. + // tsx`'s `useFocusContextService` TSDoc) — every guard read below then + // evaluates to `undefined` (falsy), i.e. "assume nothing else holds + // focus", matching every other optional-dependency fallback in this + // module rather than skipping the whole effect. + const focusContext = useFocusContextService(); + // Detects a genuine "active document changed" transition ONLY — advanced + // unconditionally the instant a new uri is seen, independent of the + // do-not-steal guard below (this was never the bug CodeRabbit found; + // keep it separate from `pendingFocusUriRef` so it stays that way). + const previousActiveUriRef = useRef(undefined); + // The uri still OWED a focus attempt, if any — set when a transition is + // detected, left alone (not cleared) if the guard defers it, and cleared + // only once `attemptFocus` actually calls `.focus()`. `undefined` means + // "nothing pending" — the common, unguarded case reaches that state on + // the very same render that detected the transition. + const pendingFocusUriRef = useRef(undefined); + + const attemptFocus = useCallback(() => { + if (!pendingFocusUriRef.current) return; // Nothing owed — including a prior success. + + // Do-not-steal guard (this component's TSDoc): the command palette, an + // input box, the find widget, or the explorer sidebar may legitimately + // hold focus right now — none of them are this component's own React + // descendants, so they can only be observed through the shared context + // service, not through props/tree structure. Left pending (not + // cleared) when guarded, so a later retry (this effect's own `uri` + // change, or the `onDidChange`-driven retry below) can pick it back up. + if ( + focusContext?.get(QUICK_PICK_FOCUS_CONTEXT_KEY) || + focusContext?.get(INPUT_BOX_FOCUS_CONTEXT_KEY) || + focusContext?.get("findWidgetFocus") || + focusContext?.get("explorerFocus") || + isFindOpen + ) { + return; + } + + // Cleared BEFORE focusing (not after): `.focus()` synchronously fires + // `FOCUSED`, which `editorView.tsx`'s `useFocusTracking("editorTextFocus")` + // reports straight into `focusContext`, which in turn fires the + // `onDidChange` this same function is subscribed to below — clearing + // first guarantees that re-entrant call sees nothing pending and no-ops, + // rather than racing a second `.focus()` call on the same node. + pendingFocusUriRef.current = undefined; + textPlaneNodeRef.current?.focus(); + }, [focusContext, isFindOpen]); + + useEffect(() => { + const uri = props.activeDocument?.uri; + if (uri === previousActiveUriRef.current) return; // No transition. + previousActiveUriRef.current = uri; + pendingFocusUriRef.current = uri; // `undefined` uri: nothing to focus (case 2's precondition). + attemptFocus(); + }, [props.activeDocument?.uri, attemptFocus]); + + // Retries a deferred attempt once whatever guarded it clears (this + // component's TSDoc's "A deferred attempt is retried, never discarded"). + // `onDidChange` fires on ANY context-service key changing, not just the + // four this guard reads — deliberately unfiltered, since `attemptFocus` + // itself is a cheap no-op whenever nothing is pending (`pendingFocusUriRef` + // is `undefined`) or the guard is still active, so over-calling it here + // costs nothing and needs no per-key wiring to stay correct as guards + // come and go. + useEffect(() => { + if (!focusContext) return undefined; + const subscription = focusContext.onDidChange(() => attemptFocus()); + return () => subscription.dispose(); + }, [focusContext, attemptFocus]); + // Stable identity across every render (CodeRabbit finding on PR #59) — a // fresh inline arrow here would give `EditorView`'s own `textPlaneRef` // `useCallback` (`editorView.tsx`, deps include `onTextPlaneNode`) a new diff --git a/requirements.md b/requirements.md index 03760ae..34625cb 100644 --- a/requirements.md +++ b/requirements.md @@ -110,6 +110,7 @@ The following points were open in the draft specification and are resolved here 4. THE core SHALL persist layout state (sidebar width, visibility) across sessions. 5. THE MVP SHALL support exactly one editor group (no split editing) with multiple tabs. 6. THE editor view SHALL be a custom component with an editor-owned cursor/selection overlay supporting multiple cursors and a line-number gutter. +7. WHEN a document becomes the active editor tab — on startup with a document already open, when the first document opens on an empty workspace, or when the user switches tabs — THE system SHALL give keyboard focus to that document's text plane so typing works immediately with no manual focus action, UNLESS the command palette, an input box, the find widget, or the explorer sidebar currently holds focus, in which case focus SHALL NOT be moved. ### Requirement 7: Theming