diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md index dd31015..ab04f8e 100644 --- a/UX_REVIEW_2026-07-27.md +++ b/UX_REVIEW_2026-07-27.md @@ -958,7 +958,146 @@ categories are the unit she can reason about — five article fixes are one decision, but her whole queue is not — and a single button that rewrites the document in one press is the opposite of a tool that teaches. -**Still open in item 8:** the keyboard triage flow. +### 8 — Keyboard triage DONE (eleventh session). The item's own keys were the one part that couldn't be built. + +This is the last of item 8, and the last small item in the review. It arrives with +half of it already built by item 7: because the anchored popover is the primary +surface in *both* layouts, one keyboard flow covers rail and no-rail, and there +was never a question of driving two. + +**The item asks for Tab/Shift+Tab "or n/p", and n/p cannot exist.** This is a +text editor. An unmodified letter is a letter, and `n` would type an n in the +middle of her sentence. Tab is nearly as bad while the caret is in the prose. +Both are fine *once a card is open and holding focus* — which is exactly where +the item asks for them — so the only real design question was how to get there, +and that needs a key that is safe to press mid-sentence. Mid-*composition*, even: +she writes Chinese, and a Chinese IME uses `,` and `.` to page its candidate +window, so the entry chord is guarded by `fromIME` like every other key Petal +binds. + +**The shape, then:** + +- `Ctrl/Cmd+.` and `Ctrl/Cmd+,` step to the next/previous underline from + anywhere in the text, which is both how triage is entered and how it is + continued. They join the existing `Ctrl+F` / `Ctrl+D` / `Ctrl+J` family in the + same handler. +- The card that opens **takes focus**, and from there the item's keys work as + written: `Tab`/`Shift+Tab` step, `Enter` accepts, `Del`/`Backspace` dismisses, + `Esc` leaves. `?` opens Ask Petal — the panel focuses its own input, and + `Esc` there steps back out to the card rather than out of triage, so the one + detour that matters to an ESL writer isn't a mouse-only feature. +- **Answering a card advances by itself.** Accept or dismiss and the next stop + opens focused; the last one closes the card and puts the caret back in the + text, just past the span she was reading about. That is the whole acceptance + criterion — a document triaged without the mouse is five presses of Enter. + +**Implemented:** + +- `triage.ts` — `stepId`, `entryId`, `idAfterRemoval`. Pure, and taking the queue + as an argument, so wrap-around, caret-relative entry and "where does an + answered card hand over to" are testable without a ProseMirror document or a + layout — the same split `acceptBatch.ts` used, for the same reason. +- **The queue is the underlines, not the suggestion list.** Read off the + decoration DOM in document order. A suggestion the editor couldn't anchor has + no underline, and a triage stop she cannot see is worse than one she never + visits; reading the DOM also guarantees every stop can be anchored, which is + what the card needs to position itself. +- `SuggestionCard.tsx` — `keyboard` mode: `tabIndex={-1}`, focus on mount *and on + every step* (stepping keeps the same component mounted and swaps the suggestion + inside it), `focus({ preventScroll: true })` for AskPetal's reason, an accent + border where a pointer would otherwise be saying "this one", and the legend. +- `EditorCore.tsx` — `orderedSpans` / `openTriageAt` / `stepTriage` / `exitTriage`, + and `queueTriageAfter`, which notes the next stop *before* the action, because + the queue has to be read while the answered card is still in it. +- An Accept-all pressed from a triage card resumes after **her** card, not after + whichever member of the batch happened to be last — the queue is in document + order and a category is scattered through it. + +**Two things the code had to be told, and both are about other people's keys.** + +- **Escape is overloaded.** App has a window listener where Escape leaves + distraction-free mode; unhandled, one press would have closed the card *and* + restored the sidebar *and* — via item 7's rail-follows-the-mode — pulled the + rail out from under her. In triage that key means "this card", never "the + writing mode", so the card stops the event. +- **The Spanish pack's own test caught the legend.** `?` in a Spanish `Line` + must open with `¿`, and the i18n suite says so for every native half in the + pack. It is right, and it is wrong here: this `?` is a key cap, no more Spanish + punctuation than `Esc`. The exemption is one named entry with the reason + written next to it, rather than a loosened rule. + +**The legend is bilingual, against the card's own convention.** Accept, Dismiss +and Ask Petal stay English because they name the thing she is learning to talk +about (item 5's reasoning, and item 8's for the Accept-all label). This isn't +that: it is an instruction for operating Petal, like the status bar, so it is +bilingual and leads with the pair language. The key *names* are what is printed +on her keyboard, so fr says `Entrée`/`Suppr`/`Échap` and es says `Intro`/`Supr` — +a legend she has to translate back to find the key is not a legend. + +**Verified in a real browser at the review's own 1517×810**, on a fresh database +with no model at all (the rule pack from item 3b needs none), against the served +bundle hash checked against `web/dist` first. Nineteen assertions on a clean run, +then the acceptance criterion itself: + +- *Entry.* Five underlines from one typed paragraph. `Ctrl+.` opened the first + card after the caret, focused, accent-bordered, legend showing both halves — + and **did not type a period into her sentence**. +- *Walking.* Tab through all five to the last, once more to wrap to the first, + Shift+Tab to wrap backwards. Every step landed on the card it should. +- *Answering.* Enter accepted and the next card opened focused by itself + (5 → 4 underlines, text corrected); Del dismissed and advanced (4 → 3, text + untouched); `?` opened Ask Petal with its input focused, and Escape there came + back to the card rather than out of triage. +- *The criterion.* From `Ctrl+.`, **five presses of Enter and nothing else**: + zero underlines left, `I want an apple and an orange. She has three cats. He + walk to an office.`, card closed, caret back in the prose, no horizontal + overflow. No mouse after the initial click into the document. +- *Both layouts.* Escape out of distraction-free (rail gone, sidebar back), then + `Ctrl+,` — a card opened, focused, on the last underline before the caret, + with no rail anywhere. +- *The mixed path.* Accept-all **clicked** while a keyboard card was open: whole + category applied, triage ended cleanly with focus in the text. No page errors + in any run. + +**No Chrome extension this session** — it wasn't connected — so the browser was +driven over CDP against a real headless Chrome instead. That turned out to be +the better tool for this item and is worth recording: `Input.dispatchKeyEvent` +produces genuine trusted keystrokes, which is the only honest way to test a +feature that *is* keystrokes. It also sidesteps the tenth session's rAF trap — +`document.visibilityState` reads `visible`, so `recomputeRail` runs. The driver +is ~70 lines (`connect` → `key`/`click`/`typeText`/`shot`/`ev`). + +**A measurement trap, and a cheap one.** The first run reported zero of +everything because the click that focused the editor was at y=300 and the empty +document's prose box ends at y=231. Nothing errored; the text simply went +nowhere. The second reported five underlines becoming three, because it reused +the *previous run's document* — where two of those spans had already been +dismissed, and item 8's own settled-spans work was correctly refusing to raise +them again. **Reset the database between browser runs**, or the feature you +shipped last session will look like the bug you're chasing this one. + +**Deliberately not done:** + +- No keyboard binding for Accept-all. Every other triage key answers the card in + front of her; a key that rewrites parts of the document she cannot see is a + different kind of decision, and it is one worth the deliberate reach for a + button. The path still works if she clicks it, and is tested. +- No visual cue in the text beyond the existing active-span glow (which the rail + already drives), and no "3 of 5" counter on the card. The status bar already + counts the queue, and a position indicator turns walking one's own mistakes + into a progress bar — the pressure this review's non-goals rule out. + +Coverage: `triage.test.ts` (wrap-around at both ends, entry from either +direction with the caret before/on/after a span, a current card that has left the +queue, single-item and empty queues, handover after one answer and after an +Accept-all swept several, never handing back the card just answered, and a card +that was never in the queue — the provisional rule-pack case from item 3b), and +an `i18n.test.ts` case that every pack names all five keys in both halves. +**The wiring itself has no unit test**, for the reason items 6, 7 and 8 all +recorded: jsdom has no layout, every rect is zero, and a test there would pass +whatever the code did. It is browser-verified only, and is written down as such. + +**Item 8 is now complete.** --- @@ -1071,6 +1210,21 @@ of it is stale until a screenshot forces a paint. Second, **the binary embeds bundle-hash check will (rightly) fail. Untouched: item 8's keyboard flow, item 3's incremental half.)* +*(Eleventh session: item 8's keyboard triage done — see the subsection under item +8. **Item 8 is finished, and item 3's incremental surfacing is the only thing +left in the whole review.** Note this session started from a `main` that had +moved on past the tenth session's note: the settled-spans and Accept-all work is +merged and pushed, alongside three later commits that were not review items (the +zh learner direction, the es pair, the IME composition guards). Two things to +carry forward. First, **a keystroke feature has to be tested with real +keystrokes**: with the Chrome extension unconnected, CDP's +`Input.dispatchKeyEvent` against a headless Chrome turned out to be the right +tool rather than a fallback — trusted events, real layout, and +`visibilityState: visible`, so the tenth session's rAF freeze doesn't apply. +Second, **reset the database between browser runs**: a second run against the +first run's document showed two underlines missing, which was not a bug but +item 8's own dismissal persistence working exactly as it should.)* + **Migration 0015 on the live database.** It rebuilds the suggestions table, so unlike 0014 it could have dropped her rows. Backed up first — and the backup had to be the whole WAL set (`petal.db`, `-wal`, `-shm` in @@ -1085,13 +1239,31 @@ CHECK, and every existing row still carrying the label she has already read (the seven `clarity` rows include the mislabelled Chinese one — by design, only new findings get the new type; if you want that card relabelled, edit the sentence). -**Suggested next (tenth session onward):** two things remain in the whole review. -**Keyboard triage** is the one to take: it is the last of item 8, and Accept All -just built half of what it needs — a category is now a thing the UI can act on in -one step, so "triage without the mouse" is mostly about driving the anchored -popover between spans. **Item 3's incremental surfacing** is the last item of any -size, and still needs a streaming `/check`. It remains the only one left that -changes how the app *feels* rather than what it can do. +**Suggested next (eleventh session onward): item 3's incremental surfacing is all +that is left of this review.** It is also the largest, and the only one that +changes how the app *feels* rather than what it can do. What it needs hasn't +changed: a streaming `/check`, which the current response shape doesn't do. What +*has* changed is that the expensive prerequisite is long since built — item 2's +chunking means the server already knows which sentences it is re-reading and +already returns cached rows for the rest, so "deliver per-chunk results as each +sentence finishes" is a transport change rather than an analysis one. The status +bar's running count ("Found 3 so far…") is the cheap half and can ship with it; +`petalsToPolish` in the packs is already the line to reuse. + +A caution before starting it: **the 250 ms rule pass already covers the felt +latency for the errors it knows** (item 3b), so the honest scope of what remains +is the LLM's own findings arriving one sentence at a time. Measure what she +actually waits for now before designing streaming for a wait that may be +noticeably shorter than the review's original 8–15 s. + +*(Superseded, kept for the reading list: the tenth session's advice.)* Two things +remained. **Keyboard triage** was the one to take: the last of item 8, with +Accept All having built half of what it needed — a category was now a thing the +UI could act on in one step, so "triage without the mouse" was mostly about +driving the anchored popover between spans. That reading was right about the +surface and wrong about the effort: the popover was ready, but the item's own key +choices (bare Tab, n/p) can't be bound in a text editor, and picking the entry +chord was the design work. *(Superseded, kept for the reading list: the ninth session's advice.)* Three things remained. **Accept All per category** was the one with real value left — diff --git a/web/src/components/Editor/EditorCore.tsx b/web/src/components/Editor/EditorCore.tsx index b9b4579..1c745ad 100644 --- a/web/src/components/Editor/EditorCore.tsx +++ b/web/src/components/Editor/EditorCore.tsx @@ -32,9 +32,11 @@ import { Typography } from './Typography' import { Composition } from './Composition' import { RewritePreview, type RewriteStatus } from './RewritePreview' import { planBatch } from './acceptBatch' +import { entryId, idAfterRemoval, stepId, type Direction, type Span } from './triage' import { api, type Suggestion, type SuggestionType, type WordInfo } from '../../api/client' import { speak, speechSupported } from '../../audio/speech' import type { SpellChecker } from '../../hooks/useSpellChecker' +import { fromIME } from '../../lib/ime' import type { Segmenter } from '../../lib/segment' import { hanziWordAt, hanziToWordInfo, hanziPinyin } from './hanziWord' import { usePack } from '../../i18n' @@ -204,6 +206,9 @@ interface HoverState { suggestion: Suggestion top: number left: number + // Opened by a keystroke rather than a pointer: the card takes focus and + // answers the triage keys itself (item 8). + keyboard?: boolean } // The inline hover gloss: the word under the resting pointer, its Chinese @@ -646,7 +651,7 @@ export function EditorCore({ }, []) const openCardFor = useCallback( - (id: string, el: HTMLElement) => { + (id: string, el: HTMLElement, keyboard = false) => { const wrapper = wrapperRef.current if (!wrapper) return const suggestion = suggestions.find((s) => s.id === id) @@ -664,7 +669,7 @@ export function EditorCore({ setHover((prev) => { // Moving to a different highlight resets any Ask Petal pin. if (prev && prev.suggestion.id !== suggestion.id) setPinned(false) - return { suggestion, top, left } + return { suggestion, top, left, keyboard } }) }, [suggestions], @@ -721,6 +726,140 @@ export function EditorCore({ const keepOpen = useCallback(() => clearTimeout(closeTimer.current), []) + // ——— Keyboard triage (item 8) ——— + // + // The whole queue can be walked, answered and left without a pointer: + // Ctrl/Cmd+. and Ctrl/Cmd+, step through the underlines from anywhere in the + // editor, and the card that opens takes focus and answers Tab / Enter / Del / + // Esc itself (SuggestionCard). It is the *anchored* card in both layouts, + // rail or no rail — item 7's finding, that the popover at the word is the + // primary surface, is what lets one keyboard flow cover both. + // + // Why a chord and not the item's bare Tab or n/p: this is a text editor, and + // an unmodified letter is a letter. Tab is available only once the card holds + // focus, which is exactly where the item asks for it, and getting there needs + // a key that is safe to press mid-sentence — in the middle of a Chinese + // composition, even, which is why the IME guard is here too. + + // Where triage lands once the open card is answered. Recorded before the + // action, because the queue has to be read while the answered card is still in + // it; consumed when the new suggestion list arrives. A `null` id means the + // queue is empty and triage is over. + const triageNextRef = useRef<{ id: string | null } | null>(null) + + // Every underline on screen, in document order, with its document position. + // The queue is the decorations rather than the suggestion list: a suggestion + // the editor couldn't anchor has no underline, and a triage stop she cannot + // see is worse than one she never visits. + const orderedSpans = useCallback((): Span[] => { + const wrapper = wrapperRef.current + if (!wrapper || !editor) return [] + const spans: Span[] = [] + const seen = new Set() + wrapper.querySelectorAll('.petal-suggestion[data-suggestion-id]').forEach((el) => { + const id = el.getAttribute('data-suggestion-id') + if (!id || seen.has(id)) return + seen.add(id) + let pos = 0 + try { + pos = editor.view.posAtDOM(el, 0) + } catch { + // A node the view no longer owns. Only the caret-relative entry point + // reads `pos`; document order is what walking uses, and that still holds. + } + spans.push({ id, pos }) + }) + return spans + }, [editor]) + + // Open a suggestion as a triage stop: scrolled to, emphasized, and focused. + // False means its underline is gone, which is triage's cue to stop. + const openTriageAt = useCallback( + (id: string): boolean => { + const el = wrapperRef.current?.querySelector( + `.petal-suggestion[data-suggestion-id="${CSS.escape(id)}"]`, + ) as HTMLElement | null + if (!el) return false + openCardFor(id, el, true) + setActiveId(id) + // The anchored card carries everything the rail card does, so expanding a + // second copy of it in the margin would be the redundancy item 7 refused. + setRailExpandedId(null) + el.scrollIntoView({ block: 'center', behavior: 'smooth' }) + return true + }, + [openCardFor], + ) + + // Leave triage and hand the keyboard back to the text, with the caret just + // past the span she was reading about — so the next thing she types continues + // the sentence she was looking at rather than wherever she last clicked. + const exitTriage = useCallback(() => { + const open = hover + closeCard() + setActiveId(null) + if (!editor) return + const range = open ? findRange(editor.state.doc, open.suggestion.original) : null + if (range) editor.chain().focus().setTextSelection(range.to).run() + else editor.commands.focus() + }, [editor, hover, closeCard]) + + const stepTriage = useCallback( + (dir: Direction) => { + const spans = orderedSpans() + if (spans.length === 0) return + const next = hover + ? stepId( + spans.map((s) => s.id), + hover.suggestion.id, + dir, + ) + : entryId(spans, editor?.state.selection.from ?? 0, dir) + if (next) openTriageAt(next) + }, + [orderedSpans, openTriageAt, hover, editor], + ) + + // Note the stop after the one being answered, while its underline is still in + // the queue. `answered[0]` is the card she is on; an Accept-all settles a whole + // category at once, and triage resumes after *her* card, not after whichever + // of the batch happened to be last. + const queueTriageAfter = useCallback( + (answered: string[]) => { + const gone = new Set(answered) + const remaining = new Set(suggestions.filter((s) => !gone.has(s.id)).map((s) => s.id)) + triageNextRef.current = { + id: idAfterRemoval( + orderedSpans().map((s) => s.id), + answered[0], + remaining, + ), + } + }, + [orderedSpans, suggestions], + ) + + // Open the next stop once the answered suggestion has actually left the list + // and the decorations have repainted around the edit. An empty queue ends + // triage the same way Escape does, which is the point at which the document is + // fully triaged and she is back in her text. + useEffect(() => { + const pending = triageNextRef.current + if (!pending) return + triageNextRef.current = null + const { id } = pending + if (!id) { + exitTriage() + return + } + requestAnimationFrame(() => { + if (!openTriageAt(id)) exitTriage() + }) + // Driven by the suggestion list alone: the callbacks are rebuilt in the same + // render, so this closure is never the stale one. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [suggestions]) + // Accept applies the replacement to the document, plays a little confetti // burst over the flagged text, then notifies the parent. The confetti is // anchored to the highlight itself (captured before the replacement removes it), @@ -748,6 +887,7 @@ export function EditorCore({ const handleAccept = useCallback( (s: Suggestion) => { let burst = burstAt(s.id) + if (hover?.keyboard) queueTriageAfter([s.id]) if (editor && s.replacement.trim() !== '') { const range = findRange(editor.state.doc, s.original) if (range) { @@ -761,7 +901,7 @@ export function EditorCore({ setRailExpandedId(null) onAccept(s) }, - [editor, onAccept, closeCard, hover, burstAt, showConfetti], + [editor, onAccept, closeCard, hover, burstAt, showConfetti, queueTriageAfter], ) // How many pending cards of each type could be accepted in one go. A card @@ -794,6 +934,14 @@ export function EditorCore({ const first = plan.steps[plan.steps.length - 1] const burst = first ? burstAt(first.suggestion.id) : null + // Resume after the card she pressed it on, not after the batch's last + // member — the queue she is walking is in document order, and the rest of + // the category may sit anywhere in it. + if (hover?.keyboard) { + const answered = settled.map((s) => s.id) + queueTriageAfter([hover.suggestion.id, ...answered.filter((id) => id !== hover.suggestion.id)]) + } + if (plan.steps.length > 0) { let chain = editor.chain().focus() for (const step of plan.steps) { @@ -806,15 +954,16 @@ export function EditorCore({ setRailExpandedId(null) onAcceptMany(settled) }, - [editor, suggestions, onAcceptMany, closeCard, burstAt, showConfetti], + [editor, suggestions, onAcceptMany, closeCard, burstAt, showConfetti, hover, queueTriageAfter], ) const handleDismiss = useCallback( (s: Suggestion) => { + if (hover?.keyboard) queueTriageAfter([s.id]) closeCard() onDismiss(s) }, - [onDismiss, closeCard], + [onDismiss, closeCard, hover, queueTriageAfter], ) // openMisspellAt resolves the word at a document position and, if the checker @@ -1265,11 +1414,21 @@ export function EditorCore({ // into the editor's content anyway) // Ctrl/Cmd+D — look up the word at the caret (the keyboard "right-click") // Ctrl/Cmd+J — rewrite the current selection more naturally (✨更自然) + // Ctrl/Cmd+. — walk to the next suggestion (Ctrl/Cmd+, for the previous), + // which is how keyboard triage is entered from the text useEffect(() => { if (!editor) return const onKey = (e: KeyboardEvent) => { if (!(e.ctrlKey || e.metaKey) || e.altKey) return const k = e.key.toLowerCase() + if (k === '.' || k === ',') { + // Chinese IMEs use , and . to page their candidate window. While one is + // open the keystroke belongs to the composition, not to Petal. + if (fromIME(e)) return + e.preventDefault() + stepTriage(k === '.' ? 1 : -1) + return + } if (k === 'f') { e.preventDefault() setFindOpen(true) @@ -1285,7 +1444,7 @@ export function EditorCore({ } window.addEventListener('keydown', onKey) return () => window.removeEventListener('keydown', onKey) - }, [editor, openWordLookup, handleRewrite]) + }, [editor, openWordLookup, handleRewrite, stepTriage]) useEffect(() => () => { clearTimeout(closeTimer.current) @@ -1422,6 +1581,9 @@ export function EditorCore({ onPointerLeave={scheduleClose} onExpandChange={setPinned} onExtent={setCardExtent} + keyboard={hover.keyboard} + onStep={stepTriage} + onExit={exitTriage} /> )} {railEnabled && railItems.length > 0 && ( diff --git a/web/src/components/Editor/SuggestionCard.tsx b/web/src/components/Editor/SuggestionCard.tsx index 20010ce..4ddb0f4 100644 --- a/web/src/components/Editor/SuggestionCard.tsx +++ b/web/src/components/Editor/SuggestionCard.tsx @@ -1,8 +1,10 @@ import { useEffect, useRef, useState } from 'react' import type { Suggestion, SuggestionType } from '../../api/client' import { usePack } from '../../i18n' +import { fromIME } from '../../lib/ime' import { AskPetal } from './AskPetal' import { TYPE_META, batchLabel, typeLabel } from './suggestionMeta' +import type { Direction } from './triage' interface Props { suggestion: Suggestion @@ -25,6 +27,13 @@ interface Props { // under it and its Accept button simply can't be reached. 0 means "nothing to // cover", which is what an unmounted card reports on its way out. onExtent?: (bottom: number) => void + // Keyboard triage (item 8). The card was opened by a keystroke rather than a + // pointer, so it takes focus and answers the keys itself: the writer is + // walking the underlines and never touches the mouse. + keyboard?: boolean + // Move to the next/previous underline, and leave triage entirely. + onStep?: (dir: Direction) => void + onExit?: () => void } // SuggestionCard is the hover panel for a single suggestion: a colored type tag, @@ -42,6 +51,9 @@ export function SuggestionCard({ onPointerLeave, onExpandChange, onExtent, + keyboard = false, + onStep, + onExit, }: Props) { const pack = usePack() const meta = TYPE_META[suggestion.type] @@ -67,6 +79,60 @@ export function SuggestionCard({ } }, [onExtent]) + // In triage the card is where the keys land, so it has to hold focus — and it + // has to re-take it on every step, because stepping keeps this same component + // mounted and only swaps the suggestion inside it. `preventScroll` for the + // reason AskPetal's input gives: the card is an absolutely-positioned overlay, + // and letting the browser "reveal" it would jump the document out from under + // the sentence she is reading. The span is scrolled to deliberately elsewhere. + useEffect(() => { + if (keyboard) cardRef.current?.focus({ preventScroll: true }) + }, [keyboard, suggestion.id]) + + // The triage keys. Only bound in keyboard mode: a card opened by the pointer + // never holds focus, and stealing Tab from one that somehow did would break + // ordinary focus movement for no gain. + function handleKeyDown(e: React.KeyboardEvent) { + if (!keyboard || fromIME(e)) return + const target = e.target as HTMLElement + // Ask Petal's question field is a text input inside this card. While it has + // focus it owns every key it can use — Tab, Enter and the letters are hers + // to type — and only Escape is taken, to step back out to the card. + const typing = target instanceof HTMLInputElement || target instanceof HTMLTextAreaElement + + if (e.key === 'Escape') { + e.preventDefault() + // Escape also leaves distraction-free mode (App's window listener), which + // would restore the sidebar and pull the rail out from under her mid- + // triage. In triage this key means "this card", or "triage" — never "the + // writing mode". + e.stopPropagation() + if (typing) cardRef.current?.focus({ preventScroll: true }) + else onExit?.() + return + } + if (typing) return + + if (e.key === 'Tab') { + e.preventDefault() + onStep?.(e.shiftKey ? -1 : 1) + } else if (e.key === 'Enter') { + // An awareness-only card has nothing to accept; Enter on it does nothing + // rather than quietly meaning something else. + if (!hasReplacement) return + e.preventDefault() + onAccept(suggestion) + } else if (e.key === 'Delete' || e.key === 'Backspace') { + e.preventDefault() + onDismiss(suggestion) + } else if (e.key === '?') { + e.preventDefault() + // Opening hands focus to the panel's own input; Escape there comes back + // here, and ? then closes it again. + toggleAsking() + } + } + function toggleAsking() { setAsking((prev) => { const next = !prev @@ -80,13 +146,18 @@ export function SuggestionCard({ ref={cardRef} role="dialog" aria-label={`${label} suggestion`} + tabIndex={keyboard ? -1 : undefined} + onKeyDown={handleKeyDown} onMouseEnter={onPointerEnter} onMouseLeave={onPointerLeave} - className="petal-suggestion-card absolute z-20 p-3.5 text-sm" + className="petal-suggestion-card absolute z-20 p-3.5 text-sm focus:outline-none" style={{ width: asking ? 340 : 300, background: 'var(--color-surface)', - border: '1px solid var(--color-border)', + // In triage the card is the only thing holding focus, and the writer has + // no pointer under it to say so. The accent border is that answer — the + // browser's own focus ring on a 300px panel reads as an error state. + border: `1px solid ${keyboard ? 'var(--color-accent)' : 'var(--color-border)'}`, borderRadius: 'var(--radius-card)', boxShadow: 'var(--shadow-soft)', ...style, @@ -161,6 +232,16 @@ export function SuggestionCard({ {batchLabel(suggestion.type, batchCount)} )} + + {keyboard && ( +
+
{pack.editor.triageHint.native}
+
{pack.editor.triageHint.en}
+
+ )} ) } diff --git a/web/src/components/Editor/triage.test.ts b/web/src/components/Editor/triage.test.ts new file mode 100644 index 0000000..548d55c --- /dev/null +++ b/web/src/components/Editor/triage.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest' +import { entryId, idAfterRemoval, stepId, type Span } from './triage' + +const spans = (...pairs: [string, number][]): Span[] => pairs.map(([id, pos]) => ({ id, pos })) + +describe('stepId — walking the queue', () => { + const order = ['a', 'b', 'c'] + + it('moves forward and backward', () => { + expect(stepId(order, 'a', 1)).toBe('b') + expect(stepId(order, 'c', -1)).toBe('b') + }) + + it('wraps at both ends, so the queue is a ring and never a dead end', () => { + expect(stepId(order, 'c', 1)).toBe('a') + expect(stepId(order, 'a', -1)).toBe('c') + }) + + it('enters at the near end when there is no current card', () => { + expect(stepId(order, null, 1)).toBe('a') + expect(stepId(order, null, -1)).toBe('c') + }) + + it('treats a card that has left the queue as no card at all', () => { + // She accepted from the rail while a triage card was open, or an edit + // dissolved the span. Stepping should still land somewhere real. + expect(stepId(order, 'gone', 1)).toBe('a') + }) + + it('has nowhere to go in an empty queue', () => { + expect(stepId([], null, 1)).toBeNull() + expect(stepId([], 'a', -1)).toBeNull() + }) + + it('stays put on a queue of one', () => { + expect(stepId(['only'], 'only', 1)).toBe('only') + expect(stepId(['only'], 'only', -1)).toBe('only') + }) +}) + +describe('entryId — where triage starts from the caret', () => { + const order = spans(['a', 10], ['b', 40], ['c', 90]) + + it('goes forward to the first underline at or after the caret', () => { + expect(entryId(order, 0, 1)).toBe('a') + expect(entryId(order, 11, 1)).toBe('b') + expect(entryId(order, 40, 1)).toBe('b') // caret sitting on the span itself + }) + + it('goes back to the last underline at or before the caret', () => { + expect(entryId(order, 100, -1)).toBe('c') + expect(entryId(order, 39, -1)).toBe('a') + expect(entryId(order, 40, -1)).toBe('b') + }) + + it('wraps rather than refusing when the caret is past every span', () => { + expect(entryId(order, 500, 1)).toBe('a') + expect(entryId(order, 0, -1)).toBe('c') + }) + + it('has nowhere to enter in an empty document', () => { + expect(entryId([], 0, 1)).toBeNull() + }) +}) + +describe('idAfterRemoval — where the answered card hands over to', () => { + const order = ['a', 'b', 'c', 'd'] + + it('carries on with the next one still standing', () => { + expect(idAfterRemoval(order, 'b', new Set(['a', 'c', 'd']))).toBe('c') + }) + + it('skips everything an Accept-all took with it', () => { + // Accept all of a category: b, c and d go together, so triage resumes at + // the only survivor rather than at a card that no longer exists. + expect(idAfterRemoval(order, 'b', new Set(['a']))).toBe('a') + }) + + it('wraps to the front when the answered card was last', () => { + expect(idAfterRemoval(order, 'd', new Set(['a', 'b', 'c']))).toBe('a') + }) + + it('ends triage when nothing is left', () => { + expect(idAfterRemoval(order, 'b', new Set())).toBeNull() + }) + + it('never hands back the card that was just answered', () => { + // The server may still be reporting it for a moment; the writer has already + // said what she thinks of it. + expect(idAfterRemoval(order, 'b', new Set(['b']))).toBeNull() + }) + + it('falls back to the whole queue when the answered card was never in it', () => { + // A provisional rule-pack card can be answered before its underline has been + // painted (item 3b's 250ms pass). There is still a queue to carry on with. + expect(idAfterRemoval(order, 'unlisted', new Set(['c', 'd']))).toBe('c') + }) +}) diff --git a/web/src/components/Editor/triage.ts b/web/src/components/Editor/triage.ts new file mode 100644 index 0000000..017b969 --- /dev/null +++ b/web/src/components/Editor/triage.ts @@ -0,0 +1,62 @@ +// Keyboard triage: walking the underlines without a mouse. +// +// The queue is the underlines themselves, in document order — not the +// suggestion list. A suggestion whose span the editor couldn't anchor has no +// underline, and a triage stop she cannot see is worse than one she never +// visits. Reading the order off the decoration DOM also means the queue is +// exactly what is on screen, which is the thing she is being asked to walk. +// +// Everything here is pure and takes the order as an argument, so the arithmetic +// (wrap-around, entry from the caret, where to land after a card is answered) +// can be tested without a ProseMirror document or a layout. + +export type Direction = 1 | -1 + +// A span in the queue: its suggestion id and where it sits in the document. +export interface Span { + id: string + pos: number +} + +// The next stop from `current`, wrapping at both ends. A `current` that is no +// longer in the queue (or absent) enters at whichever end the direction implies, +// so the first press of "next" lands on the first underline and "previous" on +// the last. +export function stepId(order: readonly string[], current: string | null, dir: Direction): string | null { + if (order.length === 0) return null + const at = current === null ? -1 : order.indexOf(current) + if (at === -1) return dir === 1 ? order[0] : order[order.length - 1] + return order[(at + dir + order.length) % order.length] +} + +// Where triage starts when it is entered from the editor rather than continued: +// the nearest underline in the direction she asked for, measured from the caret, +// so she picks up from where she is reading rather than being thrown to the top +// of a document she has scrolled halfway down. Wraps around when the caret is +// past them all, which is the same wrap `stepId` gives once she is walking. +export function entryId(order: readonly Span[], caret: number, dir: Direction): string | null { + if (order.length === 0) return null + if (dir === 1) { + for (const span of order) if (span.pos >= caret) return span.id + return order[0].id + } + for (let i = order.length - 1; i >= 0; i--) if (order[i].pos <= caret) return order[i].id + return order[order.length - 1].id +} + +// Where to land after the card she was on is answered (accepted, dismissed, or +// swept up by an Accept-all). The order is the one read *before* the action, so +// "the next one" means the next in the queue she was walking; `remaining` is +// what actually survived. Falls back to scanning forward and then round to the +// front, and returns null when nothing is left — which is triage finishing, not +// an error. +export function idAfterRemoval( + order: readonly string[], + answered: string, + remaining: ReadonlySet, +): string | null { + const at = order.indexOf(answered) + const rest = at === -1 ? order : [...order.slice(at + 1), ...order.slice(0, at)] + for (const id of rest) if (id !== answered && remaining.has(id)) return id + return null +} diff --git a/web/src/i18n/i18n.test.ts b/web/src/i18n/i18n.test.ts index 2ef9dcd..6d5e46c 100644 --- a/web/src/i18n/i18n.test.ts +++ b/web/src/i18n/i18n.test.ts @@ -203,6 +203,22 @@ describe('the zh pack', () => { expect(p.status.petalsToPolish(2).en).toContain('petals to polish') }) + // The triage legend is the only place a Petal binding is written down, so a + // pack that drops a key drops the feature for that pair: nothing else on + // screen says Tab moves to the next underline. The key caps themselves stay + // as they are printed on the keyboard, which is why the English half is not + // the interesting one — a pack may well translate "Entrée" and be right to. + it.each(PACKS)('names every triage key in both halves ($code)', (p) => { + const { native, en } = p.editor.triageHint + expect(native, `${p.code} has no pair-language triage legend`).toBeTruthy() + expect(en, `${p.code} has no English triage legend`).toBeTruthy() + expect(en).toBe('Tab next · Enter accept · Del dismiss · ? Ask Petal · Esc exit') + // Five bindings, five entries — in whatever the pack calls the keys. + expect(native.split('·'), `${p.code} lists the wrong number of keys`).toHaveLength(5) + expect(native, `${p.code} loses the Tab key`).toContain('Tab') + expect(native, `${p.code} loses the Ask Petal key`).toContain('?') + }) + it.each(PACKS)('labels every companion, tone and style ($code)', async (p) => { const { COMPANIONS } = await import('../components/Companion/companions') for (const c of COMPANIONS) { @@ -414,7 +430,12 @@ describe('the es pack', () => { } walk(es, '') expect(lines.length).toBeGreaterThan(40) + // The one exemption, and it is not a question: `triageHint` is a legend of + // key caps, and its "?" is the key she presses to ask Petal — the same + // literal printed on the keyboard, no more Spanish punctuation than "Esc". + const keyCaps = new Set(['editor.triageHint']) for (const { native, where } of lines) { + if (keyCaps.has(where)) continue if (native.includes('?')) expect(native, `${where} closes ? without ¿`).toContain('¿') if (native.includes('!')) expect(native, `${where} closes ! without ¡`).toContain('¡') } diff --git a/web/src/i18n/packs/es.ts b/web/src/i18n/packs/es.ts index abf4b3d..ef1ec3e 100644 --- a/web/src/i18n/packs/es.ts +++ b/web/src/i18n/packs/es.ts @@ -374,6 +374,11 @@ export const es: Pack = { replace: 'Reemplazar', replaceAll: 'Todo', translateLabel: 'Traducción · Translate', + // Intro, Supr, Esc — as they are printed on a Spanish keyboard. + triageHint: { + native: 'Tab siguiente · Intro aceptar · Supr descartar · ? preguntar a Petal · Esc salir', + en: 'Tab next · Enter accept · Del dismiss · ? Ask Petal · Esc exit', + }, spelling: 'Ortografía · Spelling', noSuggestions: 'Sin sugerencias · No suggestions', addToDictionary: 'Agregar al diccionario · Add to dictionary', diff --git a/web/src/i18n/packs/fr.ts b/web/src/i18n/packs/fr.ts index 465ff3e..09208ec 100644 --- a/web/src/i18n/packs/fr.ts +++ b/web/src/i18n/packs/fr.ts @@ -316,6 +316,13 @@ export const fr: Pack = { replace: 'Remplacer', replaceAll: 'Tout', translateLabel: 'Traduction · Translate', + // The key names are the ones printed on a French keyboard — Entrée, Suppr, + // Échap — not their English equivalents. A legend she has to translate back + // to find the key is not a legend. + triageHint: { + native: 'Tab suivant · Entrée accepter · Suppr ignorer · ? demander à Petal · Échap quitter', + en: 'Tab next · Enter accept · Del dismiss · ? Ask Petal · Esc exit', + }, spelling: 'Orthographe · Spelling', noSuggestions: 'Aucune suggestion · No suggestions', addToDictionary: 'Ajouter au dictionnaire · Add to dictionary', diff --git a/web/src/i18n/packs/pt-PT.ts b/web/src/i18n/packs/pt-PT.ts index 24874a2..3400422 100644 --- a/web/src/i18n/packs/pt-PT.ts +++ b/web/src/i18n/packs/pt-PT.ts @@ -294,6 +294,10 @@ export const ptPT: Pack = { replace: 'Substituir', replaceAll: 'Tudo', translateLabel: 'Tradução · Translate', + triageHint: { + native: 'Tab seguinte · Enter aceitar · Del ignorar · ? perguntar à Petal · Esc sair', + en: 'Tab next · Enter accept · Del dismiss · ? Ask Petal · Esc exit', + }, spelling: 'Ortografia · Spelling', noSuggestions: 'Sem sugestões · No suggestions', addToDictionary: 'Adicionar ao dicionário · Add to dictionary', diff --git a/web/src/i18n/packs/zh.ts b/web/src/i18n/packs/zh.ts index 55a4c2d..a1411de 100644 --- a/web/src/i18n/packs/zh.ts +++ b/web/src/i18n/packs/zh.ts @@ -205,6 +205,10 @@ export const zh: Pack = { replace: '替换', replaceAll: '全部', translateLabel: '翻译 · Translate', + triageHint: { + native: 'Tab 下一处 · Enter 采纳 · Del 忽略 · ? 问问 Petal · Esc 退出', + en: 'Tab next · Enter accept · Del dismiss · ? Ask Petal · Esc exit', + }, spelling: '拼写 · Spelling', noSuggestions: '没有建议 · No suggestions', addToDictionary: '添加到词典 · Add to dictionary', diff --git a/web/src/i18n/types.ts b/web/src/i18n/types.ts index ee2a00c..f1eaf17 100644 --- a/web/src/i18n/types.ts +++ b/web/src/i18n/types.ts @@ -195,6 +195,13 @@ export interface Pack { // opposite case — its whole subject is her own language — so it says so in // her language first. The pack holds the rendered string, separator and all. translateLabel: string + // The key map shown along the bottom of a card opened by keyboard triage. + // Unlike the card's buttons — Accept, Dismiss, Ask Petal, which stay English + // because they name the thing she is learning to talk about — this is an + // instruction for using Petal, so it is bilingual like the status bar. The + // key names themselves (Tab, Enter, Esc) are what is printed on her + // keyboard, so they don't translate. + triageHint: Line spelling: string noSuggestions: string addToDictionary: string