From bcc52592a7e8d631b86fb1101d6e91d3d4a83ca2 Mon Sep 17 00:00:00 2001 From: adnayn <167046854+uNyanda@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:53:12 +0200 Subject: [PATCH] Fix(create form): the footer describes keys the focused field does not have MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The New note form's footer was unconditional while the list of choices it describes is gated, so on arrival — focused in Name, where no list is rendered — it already claimed `↑↓ pick`. `↑↓` never moved between Name, Folder and Tags; it only ever moved through the choice rows under Folder or Tags, and Tab is the only field-level navigation. Neither was stated. `↵ create` was wrong in three of the states it appeared in. With a row highlighted, Enter picks it. In Tags, with a valid tag typed and no highlight, Enter commits the typed tag as a chip. In Tags, with text that is not a valid tag, Enter does nothing at all. Derive both hints from state the component already holds: const hasList = focused === 'folder' ? folderRows.length > 0 : focused === 'tags' ? tagRows.length > 0 : false const rowPicked = hasList && activeRow >= 0 const inTags = focused === 'tags' const enterLabel = rowPicked ? (inTags ? 'add tag' : 'choose folder') : inTags && typedTag.trim() ? (normalizeTag(typedTag) ? 'add tag' : null) : 'create note' `hasList` is load-bearing in `rowPicked`. Typing in Folder preselects row 0 so Enter picks without an arrow key first (#467), and that index outlives the row once the text filters the list away — folderRows[0] is then undefined and Enter falls through to create(). A highlight pointing at nothing is not a highlight. The null branch is a fourth state only Tags can reach. Offering it any label would promise an action Enter cannot take, so the hint is omitted and the status line carries the reason. No key handling changes, no new imports, and Tab order is untouched — the chips and rows were already tabIndex={-1}. data-keyboard-hints stays on the container so a touch shell still drops the hints while keeping Back and Create. Adds SearchCreateForm.test.ts, the first test for this component. The extension is .ts rather than .tsx because vitest.config.ts includes only src/**/*.test.ts, so a .tsx file would silently never run. 11 cases assert the full hint list per state rather than one substring, so a hint that should be absent is proven absent. --- .../src/components/SearchCreateForm.test.ts | 222 ++++++++++++++++++ .../src/components/SearchCreateForm.tsx | 45 +++- 2 files changed, 263 insertions(+), 4 deletions(-) create mode 100644 packages/app-core/src/components/SearchCreateForm.test.ts diff --git a/packages/app-core/src/components/SearchCreateForm.test.ts b/packages/app-core/src/components/SearchCreateForm.test.ts new file mode 100644 index 000000000..fdd0a1564 --- /dev/null +++ b/packages/app-core/src/components/SearchCreateForm.test.ts @@ -0,0 +1,222 @@ +// @vitest-environment jsdom + +import { act, createElement } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { NoteMeta } from '@shared/ipc' +import { useStore } from '../store' +import type { SearchCreateDraft } from '../lib/search-create' +import { SearchCreateForm } from './SearchCreateForm' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +function note(title: string, tags: string[] = []): NoteMeta { + return { + path: `inbox/${title}.md`, + title, + folder: 'inbox', + siblingOrder: 0, + createdAt: 0, + updatedAt: 0, + size: 0, + tags, + wikilinks: [], + hasAttachments: false, + assetEmbeds: [], + excerpt: '' + } +} + +const draft: SearchCreateDraft = { name: 'zzqx', folderText: '', tags: [] } + +describe('SearchCreateForm: footer hints follow the focused field', () => { + let host: HTMLDivElement + let root: Root + let originalState: ReturnType + let originalScrollIntoView: PropertyDescriptor | undefined + + beforeEach(() => { + originalState = useStore.getState() + host = document.createElement('div') + document.body.append(host) + root = createRoot(host) + originalScrollIntoView = Object.getOwnPropertyDescriptor(Element.prototype, 'scrollIntoView') + Object.defineProperty(Element.prototype, 'scrollIntoView', { configurable: true, value: vi.fn() }) + + useStore.setState({ + // The Folder list is built from the system areas rather than from these + // entries, so an empty `folders` still yields the three picker rows. The + // tags on this note are what give the Tags field something to list. + notes: [note('Alpha', ['alpha', 'beta'])], + folders: [], + systemFolderLabels: {}, + vaultSettings: null, + activeNote: null + }) + }) + + afterEach(() => { + act(() => root.unmount()) + host.remove() + document.body.innerHTML = '' + if (originalScrollIntoView) { + Object.defineProperty(Element.prototype, 'scrollIntoView', originalScrollIntoView) + } else { + delete (Element.prototype as { scrollIntoView?: unknown }).scrollIntoView + } + vi.restoreAllMocks() + useStore.setState(originalState, true) + }) + + const render = (): void => { + act(() => + root.render( + createElement(SearchCreateForm, { + draft, + onBack: vi.fn(), + onCreate: vi.fn(), + onOpenExisting: vi.fn() + }) + ) + ) + } + + const field = (id: string): HTMLInputElement => { + const el = document.querySelector(`#search-create-${id}`) + if (!el) throw new Error(`${id} input not rendered`) + return el + } + + /** The footer is the only `div` carrying the hook, so this misses the + * `Shift+↵` chip on the collision button, which is a `kbd`. */ + const hints = (): string[] => + [...document.querySelectorAll('div[data-keyboard-hints] > span')].map( + (span) => span.textContent?.replace(/\s+/g, ' ').trim() ?? '' + ) + + const focusField = (id: string): void => { + act(() => { + field(id).focus() + }) + } + + const type = (id: string, value: string): void => { + const el = field(id) + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + act(() => { + setter.call(el, value) + el.dispatchEvent(new Event('input', { bubbles: true })) + }) + } + + const press = (id: string, init: KeyboardEventInit): void => { + act(() => { + field(id).dispatchEvent( + new KeyboardEvent('keydown', { ...init, bubbles: true, cancelable: true }) + ) + }) + } + + it('names Tab and offers create note in Name, with no list keys on arrival', () => { + render() + // The form opens focused in Name, which renders no list at all, so the + // pick hint has nothing to act on and must not be offered. + expect(hints()).toEqual(['↵ create note', 'tab next field', 'esc back']) + }) + + it('offers pick and create note in Folder, where the system areas always give rows', () => { + render() + focusField('folder') + expect(hints()).toEqual(['↑↓ pick', '↵ create note', 'tab next field', 'esc back']) + }) + + it('names the folder when a row is highlighted', () => { + render() + focusField('folder') + press('folder', { key: 'ArrowDown' }) + expect(hints()).toEqual(['↑↓ pick', '↵ choose folder', 'tab next field', 'esc back']) + }) + + it('drops pick when the typed folder matches nothing', () => { + render() + focusField('folder') + // The system folders guarantee rows only until the text filters them all + // away; a path that does not exist yet is a legitimate thing to type. + type('folder', 'zzz') + expect(hints()).toEqual(['↵ create note', 'tab next field', 'esc back']) + }) + + it('offers pick and create note in Tags once the vault has tags to list', () => { + render() + focusField('tags') + expect(hints()).toEqual(['↑↓ pick', '↵ create note', 'tab next field', 'esc back']) + }) + + it('names the tag when a row is highlighted', () => { + render() + focusField('tags') + press('tags', { key: 'ArrowDown' }) + expect(hints()).toEqual(['↑↓ pick', '↵ add tag', 'tab next field', 'esc back']) + }) + + it('still says add tag once the highlight cycles off the last row', () => { + render() + // Two tags in the vault. The highlight wraps from the last row back to the + // typed value, where Enter still commits what is typed rather than + // creating the note, so the label must not fall back to `create note`. + focusField('tags') + type('tags', 'fresh') + press('tags', { key: 'ArrowDown' }) + press('tags', { key: 'ArrowDown' }) + press('tags', { key: 'ArrowDown' }) + expect(hints()).toEqual(['↑↓ pick', '↵ add tag', 'tab next field', 'esc back']) + }) + + it('keeps pick when the typed-tag row is all there is, and still says add tag', () => { + // A vault with no tags: typing leaves exactly one row, the `Add #…` one. + // Moving the highlight off it does not remove the row, so pick stays true — + // what changes is that Enter now commits the typed text instead. + useStore.setState({ notes: [note('Alpha')] }) + render() + focusField('tags') + type('tags', 'fresh') + press('tags', { key: 'ArrowDown' }) + expect(hints()).toEqual(['↑↓ pick', '↵ add tag', 'tab next field', 'esc back']) + }) + + it('offers no Enter hint at all when the typed tag is not a valid tag', () => { + render() + focusField('tags') + // Enter has nothing to do here: no row is highlighted and `normalizeTag` + // rejects the text, so it neither adds a tag nor creates the note. The + // reason is already on screen in the status line. + type('tags', '!!!') + expect(hints()).toEqual(['tab next field', 'esc back']) + expect(document.querySelector('[data-search-create-status="error"]')).not.toBeNull() + }) + + it('keeps the hardware-hint hook in every state', () => { + render() + expect(document.querySelector('div[data-keyboard-hints]')).not.toBeNull() + focusField('folder') + expect(document.querySelector('div[data-keyboard-hints]')).not.toBeNull() + type('folder', 'zzz') + expect(document.querySelector('div[data-keyboard-hints]')).not.toBeNull() + focusField('tags') + expect(document.querySelector('div[data-keyboard-hints]')).not.toBeNull() + type('tags', '!!!') + expect(document.querySelector('div[data-keyboard-hints]')).not.toBeNull() + }) + + it('keeps Back and Create reachable regardless of the hints', () => { + render() + // The hook exists so a phone shell can drop the hints while keeping the + // buttons, so they must survive every state the footer can be in. + for (const focus of [null, 'folder', 'tags'] as const) { + if (focus) focusField(focus) + const labels = [...document.querySelectorAll('button')].map((b) => b.textContent?.trim()) + expect(labels).toContain('Back') + expect(labels).toContain('Create') + } + }) +}) diff --git a/packages/app-core/src/components/SearchCreateForm.tsx b/packages/app-core/src/components/SearchCreateForm.tsx index 8ad4c1d71..6f569346c 100644 --- a/packages/app-core/src/components/SearchCreateForm.tsx +++ b/packages/app-core/src/components/SearchCreateForm.tsx @@ -150,6 +150,36 @@ export function SearchCreateForm({ ?.scrollIntoView({ block: 'nearest' }) }, [activeRow]) + // The footer has to describe the focused field, not the form in general. `↑↓` + // only ever moved through the choice rows, and the Name field has neither + // rows nor a palette-nav handler, so naming it there advertised a capability + // that was not on screen to use. The rows can also be empty even when the + // field is focusable: the system folders guarantee them in Folder, but a + // typed path matching nothing empties the list, and a vault with no tags + // leaves Tags empty until something valid is typed. + const hasList = + focused === 'folder' ? folderRows.length > 0 + : focused === 'tags' ? tagRows.length > 0 + : false + // Enter means three different things depending on the field and on whether a + // row actually stands behind the highlight, and one word cannot cover them. + // The highlight only counts while a row is behind it: typing in Folder + // preselects row 0 so Enter picks without an arrow key first (#467), and that + // index outlives the row once the text filters the list down to nothing — + // `folderRows[0]` is then `undefined` and Enter falls through to `create()`. + // + // In Tags the third case is a typed tag, which commits as a chip rather than + // creating anything, so it reads `add tag` too. Typed text that is not a + // valid tag leaves Enter with nothing to do at all, and offering it any + // label would be a promise it cannot keep, so the hint is dropped and the + // reason is left to the status line above. + const rowPicked = hasList && activeRow >= 0 + const inTags = focused === 'tags' + const enterLabel = + rowPicked ? (inTags ? 'add tag' : 'choose folder') + : inTags && typedTag.trim() ? (normalizeTag(typedTag) ? 'add tag' : null) + : 'create note' + const create = (): void => { if (!canCreate || !nameCheck.title || !destination.destination) return // Text still sitting in the Tags field counts: the user typed it and then @@ -485,11 +515,18 @@ export function SearchCreateForm({ hides every other palette footer cannot be allowed to match it (#842). A tablet with a keyboard keeps them. */}
+ {hasList && ( + + ↑↓ pick + + )} + {enterLabel && ( + + ↵ {enterLabel} + + )} - ↑↓ pick - - - ↵ create + tab next field esc back