From 21eb0bdb85ba7686699f602991a28aa88b7146b5 Mon Sep 17 00:00:00 2001 From: Nick Gomez <122398915+nick-inkeep@users.noreply.github.com> Date: Tue, 16 Jun 2026 20:36:43 -0700 Subject: [PATCH] fix(open-knowledge): deterministic arrow descent into compound jsxComponents (S1c flake) (#1905) * fix(open-knowledge): deterministic arrow descent into compound jsxComponents (S1c) S1c (selection-indicator.e2e.ts:172, 'ArrowDown into compound Callout descends into body') flaked because PM's native caret motion across the jsxComponent's isolating contentEditable=false->true boundary commits the PM selection only asynchronously via DOMObserver readback, and under load intermittently not at all, so the descent into the Callout body silently failed and the assertion timed out. Adds L2d tryEnterCompoundJsx: the bare-arrow ENTRY mirror of the existing L2c ArrowUp exit handler (tryExitCompoundJsxUp), generalized over all four arrows the way tryL0NodeSelect is. Dispatches a deterministic TextSelection at the body's first (forward) / last (backward) inner caret instead of deferring to selectVertically + browser-native motion. Wired into all four arrow handlers. Same-wave A3 parity sweep: ArrowDown/Right/Left/Up across Callout + Accordion jsxComponent types. * test(open-knowledge): update keyboard-nav catch-path contract for the L2d site The new L2d tryEnterCompoundJsx handler renamed the ArrowUp/ArrowDown handler comments that the precedent #48 structural contract anchors on, and added a fifth catch site. Re-anchor the L2 ArrowUp/Down tests to the updated comments, add an L2d contract test pinning the new catch site, and bump the catch-count floor 4->5. GitOrigin-RevId: 21a29f6dcc3aea60212a0975601ed4f3c66da766 --- ...-arrow-descent-into-compound-components.md | 5 + .../app/src/editor/block-ux/keyboard-nav.ts | 61 ++++++++++- .../keyboard-nav-catch-contract.test.ts | 17 ++- .../tests/stress/selection-indicator.e2e.ts | 100 ++++++++++++++++++ 4 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 .changeset/reliable-arrow-descent-into-compound-components.md diff --git a/.changeset/reliable-arrow-descent-into-compound-components.md b/.changeset/reliable-arrow-descent-into-compound-components.md new file mode 100644 index 00000000..693b2a5f --- /dev/null +++ b/.changeset/reliable-arrow-descent-into-compound-components.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Pressing an arrow key to move the cursor into a Callout or Accordion body now lands reliably. Previously the descent across the component boundary relied on the browser's native caret motion, which under load could intermittently fail to place the cursor inside the body (leaving the selection stuck outside). A deterministic handler now drives the descent for all four arrow directions (down, up, left, right). diff --git a/packages/app/src/editor/block-ux/keyboard-nav.ts b/packages/app/src/editor/block-ux/keyboard-nav.ts index d4206fc6..990ee76e 100644 --- a/packages/app/src/editor/block-ux/keyboard-nav.ts +++ b/packages/app/src/editor/block-ux/keyboard-nav.ts @@ -97,6 +97,59 @@ function tryExitCompoundJsxUp(editor: Editor): boolean { } } +function tryEnterCompoundJsx(editor: Editor, dir: ArrowDirection): boolean { + const { state, view } = editor; + if (!(state.selection instanceof TextSelection)) return false; + if (!state.selection.empty) return false; + if (!view.endOfTextblock(dir)) return false; + + const $head = state.selection.$head; + const isForward = dir === 'down' || dir === 'right'; + + let adj: ReturnType | null = null; + let adjPos = -1; + if (isForward) { + const afterPos = $head.after(); + if (afterPos >= state.doc.content.size) return false; + adj = state.doc.nodeAt(afterPos); + adjPos = afterPos; + } else { + const beforePos = $head.before(); + if (beforePos <= 0) return false; + const $beforePos = state.doc.resolve(beforePos); + adj = $beforePos.nodeBefore; + if (!adj) return false; + adjPos = beforePos - adj.nodeSize; + } + + if (!adj) return false; + if (adj.type.name !== 'jsxComponent') return false; + if (adj.childCount === 0) return false; + + const adjEnd = adjPos + adj.nodeSize; + + try { + const fromPos = isForward ? adjPos + 1 : adjEnd - 1; + const found = Selection.findFrom(state.doc.resolve(fromPos), isForward ? 1 : -1, true); + if (!found || !(found instanceof TextSelection)) return false; + if (found.$head.pos <= adjPos || found.$head.pos >= adjEnd) return false; + editor.view.dispatch(state.tr.setSelection(found).scrollIntoView()); + return true; + } catch (err) { + if (!(err instanceof RangeError)) throw err; + incrementJsxArrowNodeSelectFailed(dir); + console.warn( + JSON.stringify({ + event: 'jsx-component-arrow-node-select-failed', + direction: dir, + tier: 'L2d', + reason: err.message.slice(0, 500), + }), + ); + return true; + } +} + export const KeyboardNav = Extension.create({ name: 'keyboardNav', priority: 50, // lower than Suggestion plugins so they intercept Escape first (L4) @@ -124,6 +177,7 @@ export const KeyboardNav = Extension.create({ ArrowUp: ({ editor }) => { if (tryL0NodeSelect(editor, 'up')) return true; if (tryExitCompoundJsxUp(editor)) return true; + if (tryEnterCompoundJsx(editor, 'up')) return true; const { state } = editor; if (!(state.selection instanceof NodeSelection)) return false; @@ -163,6 +217,7 @@ export const KeyboardNav = Extension.create({ ArrowDown: ({ editor }) => { if (tryL0NodeSelect(editor, 'down')) return true; + if (tryEnterCompoundJsx(editor, 'down')) return true; const { state } = editor; if (!(state.selection instanceof NodeSelection)) return false; @@ -194,9 +249,11 @@ export const KeyboardNav = Extension.create({ } }, - ArrowLeft: ({ editor }) => tryL0NodeSelect(editor, 'left'), + ArrowLeft: ({ editor }) => + tryL0NodeSelect(editor, 'left') || tryEnterCompoundJsx(editor, 'left'), - ArrowRight: ({ editor }) => tryL0NodeSelect(editor, 'right'), + ArrowRight: ({ editor }) => + tryL0NodeSelect(editor, 'right') || tryEnterCompoundJsx(editor, 'right'), Enter: ({ editor }) => { const { state } = editor; diff --git a/packages/app/tests/integration/keyboard-nav-catch-contract.test.ts b/packages/app/tests/integration/keyboard-nav-catch-contract.test.ts index c190a5db..533cf2b3 100644 --- a/packages/app/tests/integration/keyboard-nav-catch-contract.test.ts +++ b/packages/app/tests/integration/keyboard-nav-catch-contract.test.ts @@ -43,7 +43,7 @@ describe('KeyboardNav catch-path structural contract (precedent #48)', () => { }); test('L2 ArrowUp catch narrows RangeError + emits counter + structured warn with tier:L2', () => { - const body = extractCatchBody(source, '// L0 + L2: Arrow Up'); + const body = extractCatchBody(source, '// L0 + L2c + L2d + L2: Arrow Up'); expect(body).toContain('err instanceof RangeError'); expect(body).toContain("incrementJsxArrowNodeSelectFailed('up')"); @@ -54,7 +54,7 @@ describe('KeyboardNav catch-path structural contract (precedent #48)', () => { }); test('L2 ArrowDown catch narrows RangeError + emits counter + structured warn with tier:L2', () => { - const body = extractCatchBody(source, '// L0 + L2: Arrow Down'); + const body = extractCatchBody(source, '// L0 + L2d + L2: Arrow Down'); expect(body).toContain('err instanceof RangeError'); expect(body).toContain("incrementJsxArrowNodeSelectFailed('down')"); @@ -75,10 +75,21 @@ describe('KeyboardNav catch-path structural contract (precedent #48)', () => { expect(body).toContain('reason:'); }); + test('L2d tryEnterCompoundJsx catch narrows RangeError + emits counter + structured warn with tier:L2d', () => { + const body = extractCatchBody(source, 'function tryEnterCompoundJsx'); + + expect(body).toContain('err instanceof RangeError'); + expect(body).toContain('incrementJsxArrowNodeSelectFailed(dir)'); + expect(body).toContain("'jsx-component-arrow-node-select-failed'"); + expect(body).toContain('direction: dir,'); + expect(body).toContain("tier: 'L2d',"); + expect(body).toContain('reason:'); + }); + test('every catch in keyboard-nav.ts narrows to RangeError (no bare catch widening)', () => { const catchPattern = /catch\s*(?:\(\s*\w+\s*\)\s*)?\{/g; const matches = [...source.matchAll(catchPattern)]; - expect(matches.length).toBeGreaterThanOrEqual(4); // L0 + L2c + L2 up + L2 down + expect(matches.length).toBeGreaterThanOrEqual(5); // L0 + L2 up + L2 down + L2c + L2d for (const m of matches) { const window = source.slice(m.index ?? 0, (m.index ?? 0) + 1000); diff --git a/packages/app/tests/stress/selection-indicator.e2e.ts b/packages/app/tests/stress/selection-indicator.e2e.ts index 19899041..60aaf341 100644 --- a/packages/app/tests/stress/selection-indicator.e2e.ts +++ b/packages/app/tests/stress/selection-indicator.e2e.ts @@ -40,6 +40,46 @@ async function selectFirstJsxComponent(page: Page, componentName: string) { }, componentName); } +/** True when ProseMirror's selection head sits inside a jsxComponent — i.e. the + * caret descended into a compound block's body. Mirrors the inline walk S1c + * uses; shared by the L2d descent-parity tests (S1c-R/L/U/ACC). */ +async function caretInsideCompound(page: Page): Promise { + return await page.evaluate(() => { + const editor = window.__activeEditor; + if (!editor) return false; + const sel = editor.state.selection; + if (!('$head' in sel)) return false; + const $head = sel.$head as { depth: number; node: (d: number) => { type: { name: string } } }; + for (let d = $head.depth; d >= 0; d--) { + if ($head.node(d).type.name === 'jsxComponent') return true; + } + return false; + }); +} + +/** Place the caret at the start or end of the first textblock whose text equals + * `text`, via the editor API + DOM-focus commit (the S1f/S1g pattern — click + + * Home/End was flaky on loaded CI workers). */ +async function caretAtTextblock(page: Page, text: string, edge: 'start' | 'end'): Promise { + await page.evaluate( + ({ text, edge }) => { + const editor = window.__activeEditor; + if (!editor) return; + let pos = -1; + editor.state.doc.descendants((node, p) => { + if (node.type.name === 'heading' && node.textContent === text) { + pos = edge === 'start' ? p + 1 : p + 1 + node.textContent.length; + } + return true; + }); + if (pos >= 0) editor.chain().focus().setTextSelection(pos).run(); + }, + { text, edge }, + ); + await focusEditor(page); + await waitForPmSelectionInNode(page, 'heading'); +} + test('S1: ArrowDown auto-NodeSelects self-closing Callout below the cursor', async ({ page, api, @@ -223,6 +263,66 @@ test('S1g: ArrowLeft auto-NodeSelects self-closing Callout to the left of the cu await expect(callout).toHaveAttribute('data-selection-origin', 'keyboard'); }); +test('S1c-R: ArrowRight descends into compound Callout body (L2d horizontal)', async ({ + page, + api, +}) => { + await setupDoc(page, api, '# Title\n\n\n\nbody content\n\n\n'); + await page.waitForSelector('.jsx-component-wrapper[data-component-type="callout"]'); + await caretAtTextblock(page, 'Title', 'end'); + + await page.keyboard.press('ArrowRight'); + + await expect(page.locator('.jsx-component-wrapper[data-selected="true"]')).toHaveCount(0); + await waitForPmSelectionInNode(page, 'jsxComponent'); + expect(await caretInsideCompound(page)).toBe(true); +}); + +test('S1c-L: ArrowLeft descends into compound Callout body (L2d horizontal)', async ({ + page, + api, +}) => { + await setupDoc(page, api, '\n\nbody content\n\n\n\n# Footer\n'); + await page.waitForSelector('.jsx-component-wrapper[data-component-type="callout"]'); + await caretAtTextblock(page, 'Footer', 'start'); + + await page.keyboard.press('ArrowLeft'); + + await expect(page.locator('.jsx-component-wrapper[data-selected="true"]')).toHaveCount(0); + await waitForPmSelectionInNode(page, 'jsxComponent'); + expect(await caretInsideCompound(page)).toBe(true); +}); + +test('S1c-U: ArrowUp descends into compound Callout body from below (L2d vertical)', async ({ + page, + api, +}) => { + await setupDoc(page, api, '\n\nbody content\n\n\n\n# Footer\n'); + await page.waitForSelector('.jsx-component-wrapper[data-component-type="callout"]'); + await caretAtTextblock(page, 'Footer', 'start'); + + await page.keyboard.press('ArrowUp'); + + await expect(page.locator('.jsx-component-wrapper[data-selected="true"]')).toHaveCount(0); + await waitForPmSelectionInNode(page, 'jsxComponent'); + expect(await caretInsideCompound(page)).toBe(true); +}); + +test('S1c-ACC: ArrowDown descends into compound Accordion body (L2d type parity)', async ({ + page, + api, +}) => { + await setupDoc(page, api, '# Title\n\n\n\nbody content\n\n\n'); + await page.waitForSelector('.jsx-component-wrapper[data-component-type="accordion"]'); + await caretAtTextblock(page, 'Title', 'end'); + + await page.keyboard.press('ArrowDown'); + + await expect(page.locator('.jsx-component-wrapper[data-selected="true"]')).toHaveCount(0); + await waitForPmSelectionInNode(page, 'jsxComponent'); + expect(await caretInsideCompound(page)).toBe(true); +}); + test('S2: NodeSelection on a Callout emits data-selected=true on its wrapper', async ({ page, api,