From e91aa5cd3fb845d329e3ebcc2e7c2585330a8063 Mon Sep 17 00:00:00 2001 From: adnayn <167046854+uNyanda@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:22:12 +0200 Subject: [PATCH] Fix(quick-capture): the chords Vim swallows now reach their handlers Quick capture renders as its own window root, so App.tsx and its capture-phase handler are never loaded there. Key handling was a plain window listener in the bubble phase, and codemirror-vim consumes Ctrl+P, Ctrl+N and Ctrl+W as normal-mode chords and stops propagation. Three of the four shortcuts the footer advertises were dead in exactly the mode they matter in. The main window already fixed this twice (#510, #579); neither fix reached this window. A capture-phase listener now claims those chords, and only while no overlay is open. Inside an overlay its input has focus and the palette's own isPalettePreviousKey/isPaletteNextKey branch navigates, so standing aside there is what keeps Ctrl+P navigating rather than closing. That is the main window's behaviour, and Ctrl+N always worked there because the overlay guard returns early before newNoteHere. Mod+W needed different treatment, because useCloseWindowShortcut is shared with the main window. It takes a capture flag that only this call site sets; the main window leaves it off, since it already resolves that chord itself and doing both would double-handle it. Three further defects in the same window, all found while testing the first: - The picker rows put the filename last behind the path, so the filename truncated to a stub while the path stayed readable. This is #635, whose fix landed in Open Buffers and never reached here. The row layout now mirrors BufferPalette, and the path column shows only the subpath, since the folder is already labelled on the left and the filename sits beside it. - Neither overlay scrolled the selection into view, so moving past the bottom of the list left the highlight off screen. Both now do, keyed off data-qc-idx to stay clear of SearchPalette's data-search-idx. - The footer is now state-aware, and the command overlay no longer claims Mod+N/Mod+P on rows whose chords move the selection instead. The navigation hint reads Ctrl, not Mod, and that is load-bearing rather than stylistic. isPaletteNextKey and isPalettePreviousKey match on ctrlKey and reject metaKey, so Cmd+N/Cmd+P navigate in no palette on any platform. Every other palette hardcodes the literal for that reason. An earlier draft used modKey here, which would have rendered a Cmd hint that does nothing on macOS. 11 tests added. They dispatch on document for the chords that open or create, so the capture path runs while the editor never sees the key, and on the focused input for navigation inside an overlay. Each puts Vim into normal mode first, since insert mode is where the bug does not reproduce. --- .../src/components/QuickCaptureApp.test.ts | 241 ++++++++++++++++++ .../src/components/QuickCaptureApp.tsx | 145 ++++++++--- packages/app-core/src/lib/close-shortcut.ts | 17 +- 3 files changed, 368 insertions(+), 35 deletions(-) diff --git a/packages/app-core/src/components/QuickCaptureApp.test.ts b/packages/app-core/src/components/QuickCaptureApp.test.ts index ed18c4a3..815250e9 100644 --- a/packages/app-core/src/components/QuickCaptureApp.test.ts +++ b/packages/app-core/src/components/QuickCaptureApp.test.ts @@ -28,6 +28,10 @@ describe('Quick Capture Escape handling (#765)', () => { configurable: true, value: () => new DOMRect() }) + Object.defineProperty(Element.prototype, 'scrollIntoView', { + configurable: true, + value: vi.fn() + }) Object.defineProperty(window, 'matchMedia', { configurable: true, value: vi.fn(() => ({ matches: false, addEventListener: vi.fn(), removeEventListener: vi.fn() })) @@ -225,6 +229,10 @@ describe('Quick Capture close shortcut (#893)', () => { configurable: true, value: () => new DOMRect() }) + Object.defineProperty(Element.prototype, 'scrollIntoView', { + configurable: true, + value: vi.fn() + }) Object.defineProperty(window, 'matchMedia', { configurable: true, value: vi.fn(() => ({ matches: false, addEventListener: vi.fn(), removeEventListener: vi.fn() })) @@ -293,6 +301,239 @@ describe('Quick Capture close shortcut (#893)', () => { expect(host.querySelector('input')).not.toBeNull() }) + // The chords below are claimed in a capture-phase listener on window, because + // codemirror-vim consumes Mod+N / Mod+P / Mod+W as normal-mode chords and stops + // propagation, so those chords only reach the bubble listener in insert mode. + // The fix moved them into a capture-phase listener on window. Dispatching on + // document is what makes these tests exercise that phase: the capture path + // still starts at window, so the listener runs, while the editor never sees + // the key and Vim cannot stand in for it. + const onWindow = (init: KeyboardEventInit): Promise => press(document, init) + const modP: KeyboardEventInit = { key: 'p', code: 'KeyP', keyCode: 80, metaKey: true } + const modN: KeyboardEventInit = { key: 'n', code: 'KeyN', keyCode: 78, metaKey: true } + const modShiftP: KeyboardEventInit = { + key: 'p', code: 'KeyP', keyCode: 80, metaKey: true, shiftKey: true + } + // Navigation inside an overlay is Ctrl-only. isPalettePreviousKey requires + // ctrlKey and rejects metaKey outright, on every platform, which is why the + // footer reads Ctrl and not Mod. Mod+N/P only open the overlays. + const ctrlP: KeyboardEventInit = { key: 'p', code: 'KeyP', keyCode: 80, ctrlKey: true } + const ctrlN: KeyboardEventInit = { key: 'n', code: 'KeyN', keyCode: 78, ctrlKey: true } + + const overlayInput = (): HTMLInputElement | null => + host.querySelector('input') + const footerHints = (): string => + (host.querySelector('footer')?.textContent ?? '').replace(/\s+/g, ' ').trim() + const rows = (): HTMLButtonElement[] => + [...host.querySelectorAll('[data-qc-idx]')] + // Exact class token, not a substring match: the inactive row carries + // `hover:bg-paper-200/60`, which contains "bg-paper-200" as a substring. + const activeRow = (): HTMLButtonElement => + rows().find((r) => r.classList.contains('bg-paper-200')) ?? rows()[0] + + /** `window.zen` is installed by defineProperty without `writable`, so seeding + * notes has to go through the same door rather than by assignment. */ + function seedNotes(notes: { path: string; title: string; folder: string }[]): void { + const zen = (window as unknown as { zen: Record }).zen + Object.defineProperty(window, 'zen', { + configurable: true, + value: { + ...zen, + listNotes: vi.fn(async () => + notes.map((n, i) => ({ + ...n, + siblingOrder: i, + createdAt: 0, + updatedAt: 0, + size: 0, + tags: [], + wikilinks: [], + hasAttachments: false, + assetEmbeds: [], + excerpt: '' + })) + ) + } + }) + } + + /** CodeMirror starts in insert mode here, so one Escape puts Vim in normal + * mode, which is the state where the chords used to be swallowed. */ + async function toNormalMode(view: EditorView): Promise { + await press(view.contentDOM, { key: 'Escape', keyCode: 27 }) + } + + it('opens the note picker on Mod+P in normal mode, where Vim would take it', async () => { + const view = await mount() + await toNormalMode(view) + + await onWindow(modP) + + expect(overlayInput()?.placeholder).toContain('Search notes') + // Vim's cursor-up never ran, and nothing was saved or closed on the way. + expect(windowClose).not.toHaveBeenCalled() + expect(writeNote).not.toHaveBeenCalled() + }) + + it('starts a new note on Mod+N in normal mode', async () => { + const view = await mount() + await toNormalMode(view) + + await onWindow(modN) + + expect(createNote).toHaveBeenCalled() + expect(overlayInput()).toBeNull() + }) + + it('hides the window on Mod+W in normal mode, keeping the draft', async () => { + const view = await mount() + await toNormalMode(view) + + await onWindow(cmdW) + + expect(windowClose).toHaveBeenCalledOnce() + }) + + it('opens the command palette on Mod+Shift+P in normal mode', async () => { + const view = await mount() + await toNormalMode(view) + + await onWindow(modShiftP) + + expect(overlayInput()?.placeholder).toContain('Run a command') + }) + + it('moves down rather than toggling once the picker has focus', async () => { + seedNotes([ + { path: 'inbox/Alpha.md', title: 'Alpha', folder: 'inbox' }, + { path: 'inbox/Beta.md', title: 'Beta', folder: 'inbox' } + ]) + await mount() + await onWindow(modP) + expect(rows()).toHaveLength(2) + expect(activeRow().textContent).toContain('Alpha') + + // The chord targets the focused input, which is where a real keystroke + // lands. The window capture listener stands aside while an overlay is + // open, so the overlay's own handler navigates rather than the chord + // re-entering the capture handler and closing the picker. + await press(overlayInput()!, ctrlN) + + expect(overlayInput()?.placeholder).toContain('Search notes') + expect(rows()).toHaveLength(2) + expect(activeRow().textContent).toContain('Beta') + }) + + it('holds at the top row on Ctrl+P instead of closing the picker', async () => { + seedNotes([ + { path: 'inbox/Alpha.md', title: 'Alpha', folder: 'inbox' }, + { path: 'inbox/Beta.md', title: 'Beta', folder: 'inbox' } + ]) + await mount() + await onWindow(modP) + const input = overlayInput()! + await press(input, ctrlN) // move off the top row first + + // Ctrl+P is previous. From the top row it clamps rather than toggling the + // overlay shut, which is the specific failure being fixed. + await press(input, ctrlP) + expect(overlayInput()?.placeholder).toContain('Search notes') + expect(activeRow().textContent).toContain('Alpha') + + // And it does go up when there is somewhere to go. + await press(input, ctrlN) + await press(input, ctrlP) + expect(activeRow().textContent).toContain('Alpha') + }) + + it('stops moving at the last row instead of wrapping or closing', async () => { + seedNotes([ + { path: 'inbox/Alpha.md', title: 'Alpha', folder: 'inbox' }, + { path: 'inbox/Beta.md', title: 'Beta', folder: 'inbox' } + ]) + await mount() + await onWindow(modP) + const input = overlayInput()! + await press(input, ctrlN) // onto Beta, the last row + await press(input, ctrlN) // one past the end + + expect(rows()).toHaveLength(2) + expect(activeRow().textContent).toContain('Beta') + }) + + it('swaps the footer hints while an overlay is open', async () => { + await mount() + const idle = footerHints() + expect(idle).toContain('notes') + expect(idle).toContain('close') + + await onWindow(modP) + const open = footerHints() + expect(open).toContain('move') + expect(open).toContain('esc close') + // Ctrl, not Mod. Cmd+N/P do not navigate in any palette, so advertising + // them here would be the same lie Bug B was about, reintroduced. + expect(open).toContain('Ctrl+N/P') + expect(open).not.toContain('⌘N/P') + // The chords that no longer apply inside an overlay are not offered. + expect(open).not.toContain('new') + expect(open).not.toContain('cmd') + }) + + it('drops the two command hints that no longer run inside the command palette', async () => { + await mount() + await onWindow(modShiftP) + + const listed = rows().map((r) => r.textContent?.replace(/\s+/g, ' ').trim() ?? '') + expect(listed.some((t) => t.startsWith('Save and start a new note'))).toBe(true) + expect(listed.some((t) => t.startsWith('Open another note'))).toBe(true) + // Mod+Enter is not a navigation chord, so its hint stays. + expect(rows()[0].textContent).toContain('↩') + }) + + it('scrolls the active row into view in both overlays', async () => { + seedNotes([ + { path: 'inbox/Alpha.md', title: 'Alpha', folder: 'inbox' }, + { path: 'inbox/Beta.md', title: 'Beta', folder: 'inbox' } + ]) + const scrolled = Element.prototype.scrollIntoView as unknown as ReturnType + + await mount() + + await onWindow(modP) + scrolled.mockClear() + await press(overlayInput()!, ctrlN) + expect(activeRow().textContent).toContain('Beta') + expect(scrolled).toHaveBeenCalledWith({ block: 'nearest' }) + + // Dismiss, then the same check for the command overlay. Its effect is + // keyed on the same `active`, but it is a separate component. + await press(overlayInput()!, { key: 'Escape', keyCode: 27 }) + await onWindow(modShiftP) + scrolled.mockClear() + await press(overlayInput()!, ctrlN) + expect(scrolled).toHaveBeenCalledWith({ block: 'nearest' }) + }) + + it('shows only the subpath beside the filename', async () => { + seedNotes([ + { + path: 'inbox/ZenNotes/Enhancements/Calendar Panel Tasks Placement.md', + title: 'Calendar Panel Tasks Placement', + folder: 'inbox' + } + ]) + await mount() + await onWindow(modP) + + const cells = [...rows()[0].querySelectorAll('span')].map((s) => s.textContent?.trim() ?? '') + expect(cells).toContain('Calendar Panel Tasks Placement') + expect(cells).toContain('ZenNotes/Enhancements') + // Neither the folder nor the filename is repeated in the path column. + expect(cells.filter((c) => c === 'inbox')).toHaveLength(1) + expect(cells.some((c) => c.endsWith('.md'))).toBe(false) + }) + it('follows a rebind of Close active tab', async () => { localStorage.setItem( 'zen:prefs:v2', diff --git a/packages/app-core/src/components/QuickCaptureApp.tsx b/packages/app-core/src/components/QuickCaptureApp.tsx index 95b082e3..684d529b 100644 --- a/packages/app-core/src/components/QuickCaptureApp.tsx +++ b/packages/app-core/src/components/QuickCaptureApp.tsx @@ -576,7 +576,7 @@ export function QuickCaptureApp(): JSX.Element { // save, the draft stays in this renderer for the next show. The picker and // the command palette keep it, as palettes do in the main window. const overlayOpen = useCallback(() => overlayRef.current !== 'none', []) - useCloseWindowShortcut(overlayOpen) + useCloseWindowShortcut(overlayOpen, true) const submitAndCloseRef = useRef(submitAndClose) useEffect(() => { submitAndCloseRef.current = submitAndClose @@ -594,21 +594,8 @@ export function QuickCaptureApp(): JSX.Element { void submitAndCloseRef.current() return } - if (mod && !e.shiftKey && !e.altKey && e.key.toLowerCase() === 'n') { - e.preventDefault() - void newNoteRef.current() - return - } - if (mod && e.shiftKey && e.key.toLowerCase() === 'p') { - e.preventDefault() - setOverlay((current) => (current === 'command' ? 'none' : 'command')) - return - } - if (mod && !e.shiftKey && !e.altKey && e.key.toLowerCase() === 'p') { - e.preventDefault() - setOverlay((current) => (current === 'search' ? 'none' : 'search')) - return - } + // Mod+N, Mod+P and Mod+Shift+P are handled in the capture-phase listener + // below, which stands aside while an overlay is open. if (e.key === 'Escape') { // CodeMirror can consume Esc (e.g. collapsing a visual selection) // without stopping propagation. Do not also save and hide. (#765) @@ -631,6 +618,40 @@ export function QuickCaptureApp(): JSX.Element { return () => window.removeEventListener('keydown', handler) }, []) + // codemirror-vim consumes Ctrl+P, Ctrl+N and Ctrl+W as normal-mode chords and + // stops propagation, so the bubble handler above never sees them and the + // footer advertises shortcuts that only work in insert mode. Capture phase, + // the same treatment App.tsx gives its own editor chords (#510, #579). + // + // Gated on no overlay being open: with one open its input has focus and its + // own isPalettePreviousKey branch moves the selection, so standing aside here + // is what keeps Ctrl+P navigating instead of closing. + // + // Escape deliberately stays in the bubble handler, whose branch reads + // e.defaultPrevented on the grounds that CodeMirror has already run (#765). + useEffect(() => { + const overlayChords = (e: KeyboardEvent): void => { + if (overlayRef.current !== 'none') return + if (!(e.metaKey || e.ctrlKey) || e.altKey) return + const key = e.key.toLowerCase() + if (e.shiftKey && key === 'p') { + e.preventDefault() + e.stopImmediatePropagation() + setOverlay((current) => (current === 'command' ? 'none' : 'command')) + } else if (!e.shiftKey && key === 'n') { + e.preventDefault() + e.stopImmediatePropagation() + void newNoteRef.current() + } else if (!e.shiftKey && key === 'p') { + e.preventDefault() + e.stopImmediatePropagation() + setOverlay((current) => (current === 'search' ? 'none' : 'search')) + } + } + window.addEventListener('keydown', overlayChords, true) + return () => window.removeEventListener('keydown', overlayChords, true) + }, []) + const isMacPlatform = useMemo(() => { try { return window.zen.platformSync() === 'darwin' @@ -769,15 +790,36 @@ export function QuickCaptureApp(): JSX.Element { {modKey}↩ save - - {modKey}N new - - - {modKey}P notes - - - {modKey}⇧P cmd - + {overlay === 'none' ? ( + <> + + {modKey}N new + + + {modKey}P notes + + + {modKey}⇧P cmd + + + {modKey}W close + + + ) : ( + <> + + {/* Ctrl, not Mod: isPaletteNextKey/isPalettePreviousKey + accept Ctrl+N/P and Ctrl+J/K only, with no Cmd branch. + Same literal string the other palettes use, so the footer + stays true on macOS where Mod+N/P does not navigate. */} + ↑↓{' '} + Ctrl+N/P move + + + esc close + + + )} @@ -802,6 +844,17 @@ interface NotePickerOverlayProps { onCancel: () => void } +/** Where a note sits inside its folder, with neither the folder nor the + * filename repeated. The row already shows the folder on the left and the + * title next to it, so a full path would say the same thing twice and leave + * the title nothing but a truncated stub. */ +function noteSubpath(note: Pick): string { + const within = note.path.startsWith(`${note.folder}/`) + ? note.path.slice(note.folder.length + 1) + : note.path + return within.replace(/[^/]+$/, '').replace(/\/+$/, '') +} + function NotePickerOverlay({ notes, onPick, onCancel }: NotePickerOverlayProps): JSX.Element { const [query, setQuery] = useState('') const [active, setActive] = useState(0) @@ -811,6 +864,12 @@ function NotePickerOverlay({ notes, onPick, onCancel }: NotePickerOverlayProps): inputRef.current?.focus() }, []) + const listRef = useRef(null) + useEffect(() => { + const el = listRef.current?.querySelector(`[data-qc-idx="${active}"]`) + el?.scrollIntoView({ block: 'nearest' }) + }, [active]) + const searchIndex = useMemo(() => buildNoteSearchIndex(notes), [notes]) const results = useMemo(() => { @@ -858,7 +917,7 @@ function NotePickerOverlay({ notes, onPick, onCancel }: NotePickerOverlayProps): className="w-full bg-transparent text-sm outline-none placeholder:text-ink-400" /> -
+
{results.length === 0 ? (
No notes match. Esc to dismiss. @@ -869,6 +928,7 @@ function NotePickerOverlay({ notes, onPick, onCancel }: NotePickerOverlayProps): return ( ) }) @@ -912,6 +985,12 @@ function CommandOverlay({ modKey, mode, vimMode, onAction, onCancel }: CommandOv inputRef.current?.focus() }, []) + const listRef = useRef(null) + useEffect(() => { + const el = listRef.current?.querySelector(`[data-qc-idx="${active}"]`) + el?.scrollIntoView({ block: 'nearest' }) + }, [active]) + const all = useMemo( () => [ { @@ -927,15 +1006,18 @@ function CommandOverlay({ modKey, mode, vimMode, onAction, onCancel }: CommandOv keywords: 'save write keep open' }, { + // This overlay only mounts while it is open, and while it is open those + // two chords move the selection rather than running the command. Mod+Enter + // is not a navigation chord, so its hint stays. id: 'new' as CommandAction, label: 'Save and start a new note', - hint: `${modKey}N`, + hint: '', keywords: 'new fresh next another note save' }, { id: 'open' as CommandAction, label: 'Open another note…', - hint: `${modKey}P`, + hint: '', keywords: 'open switch picker find search note' } ], @@ -986,7 +1068,7 @@ function CommandOverlay({ modKey, mode, vimMode, onAction, onCancel }: CommandOv className="w-full bg-transparent text-sm outline-none placeholder:text-ink-400" />
-
+
{results.length === 0 ? (
No commands match.
) : ( @@ -995,6 +1077,7 @@ function CommandOverlay({ modKey, mode, vimMode, onAction, onCancel }: CommandOv return (