diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md index 89b4916..dd31015 100644 --- a/UX_REVIEW_2026-07-27.md +++ b/UX_REVIEW_2026-07-27.md @@ -861,6 +861,105 @@ one from many. **Still open in item 8:** Accept All per category, and the keyboard triage flow. Both untouched. +### 8 — Accept All per category DONE (tenth session). The undo step decided the shape. + +The item's one design question — the batch must be a single undo — turned out to +decide the implementation rather than follow it. Five accepts is five +transactions, and prosemirror-history would group them only by *timing*, which is +not a guarantee. So every replacement goes into one Tiptap `chain()`: a chain is +applied as a single transaction, and the history plugin undoes a transaction as +one event. That is the whole mechanism, and the rest of the work is making a set +of spans safe to apply in one go. + +**Implemented:** + +- `acceptBatch.ts` — `planBatch(list, resolve)`, pure, takes the span resolver as + an argument so the arithmetic can be tested without a ProseMirror document. + Every span is resolved against the doc as it stands *before* any edit, and the + steps are returned **last-span-first**: applying from the end backwards means no + earlier position can be shifted by a later replacement, which is what lets them + share one transaction with no position mapping. +- Three outcomes, not one. A card whose span is **gone** (she fixed it herself) is + settled without an edit — exactly what a single Accept already does with an + unresolvable span. A card that **overlaps** one already taken is left on screen: + `findRange` resolves to the *first* occurrence, so two cards quoting the same + words would have the second overwrite the first. A batch that silently dropped + either would be reporting edits it never made. +- `EditorCore.handleAcceptAll(type)` — builds the plan, applies the chain, fires + **one** confetti burst over the topmost changed span (read before the edit + removes the highlight), and hands the settled cards to the parent. + `burstAt`/`showConfetti` were lifted out of `handleAccept`, which now shares + them. +- `App.handleAcceptMany` — the bookkeeping only. Each row is filed individually + (there is no batch endpoint, and each accept plants its own word in the garden) + but the kitten cheers **once**: five cheers for one click would read as five + separate congratulations for a decision she made once. +- `batchLeaders` — which card carries the control. The item asks for a category + *header* in the rail, and the rail cannot have one: cards are anchored to their + own sentence, so a type's cards are scattered down the column. The closest + honest stand-in is the first card of its kind. The first version put the button + on every card of the type, which in the browser was five identical buttons in + one column saying one thing. +- The control appears only at **two or more** — a lone Grammar card doesn't grow a + second button saying the same thing as the first — and on both surfaces, since + item 7 made the anchored popover primary in both layouts. It is outlined in the + category's colour rather than filled like Accept: it acts on cards she can't see + from where she's standing, and an equally loud button would invite the click she + meant to give the one suggestion in front of her. +- The label stays English (`Accept all Tidy-up (4)`) even on a translation card, + whose pill is bilingual. The pill names the kind of advice she is reading; this + names an action over the rest of the queue, and every other word in that row — + Accept, Dismiss, Ask Petal — is English. A control that changed language between + cards would read as a different control. + +**Verified in a real browser at the review's own 1517×810**, against a stub model +server (no VPN, no GPU) that adds two `grammar` findings so the document carries +two categories at once: + +- *Single undo, measured.* `view.dispatch` hooked to count doc-changing + transactions: accepting five Tidy-up cards produced **one** transaction and took + the history's `done` count from 1 to 2. One Ctrl+Z restored all five originals; + one Ctrl+Shift+Z put all five corrections back. +- *Categories don't touch each other.* With three Tidy-up and one Grammar card, + "Accept all Tidy-up (3)" fixed exactly its three; the Grammar span stayed + underlined and its card stayed put. +- *One control per category*, on the first card of its kind, on the rail — and the + Grammar card, alone in its type, offered none on either surface. +- *Both surfaces click.* The rail's button and the anchored popover's own button + were each clicked and each applied the whole category, closing the card and + leaving one cheer behind. + +**A measurement trap that cost an hour, and it is the sixth and ninth sessions' +trap in a third disguise.** After the accept the rail appeared to keep its +accepted cards on screen indefinitely — server said nothing pending, the status +bar agreed, the DOM still had five cards. It is not a bug: **`recomputeRail` does +its work inside `requestAnimationFrame`, and Chrome does not fire rAF in a hidden +tab.** Driving the page through the JS tool leaves the window backgrounded, so +the rail is simply frozen; every screenshot (which forces a paint) showed it +correctly empty. Anything measured through rAF is invalid unless the tab is +foreground — check `document.visibilityState` before believing a rail reading. + +And the bundle-hash check caught its third variant: **the Go binary embeds +`web/dist`**, so rebuilding the frontend alone changes nothing. `npm run build` +without `go build` served the previous bundle from a server that had just been +restarted and looked entirely healthy. + +Coverage: `acceptBatch.test.ts` — the ordering property asserted by actually +applying the plan to a string and checking the result; document order rather than +card order; the missing-span case; two cards quoting the same words; adjacent +spans not counting as overlapping; an awareness-only card with nothing to insert; +and `batchLeaders` (one leader per category, follows the stack order it is given, +empty stack). **The wiring itself has no unit test**, for the reason items 6 and 7 +recorded: jsdom has no layout, and a rail test there would pass whatever the code +did. It is browser-verified only, and is written down as such. + +**Deliberately not done:** no "accept everything" across all categories. The +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. + --- ## Explicit non-goals (from this review) @@ -960,6 +1059,18 @@ unbound and the old bundle keeps serving — the eighth session's lesson with a different cause, and the same bundle-hash check catches it. Untouched: item 8's Accept All and keyboard flow, item 3's incremental half.)* +*(Tenth session: item 8's Accept All per category done — see the subsection under +item 8. **Still not pushed and not deployed**, and the undeployed stack is now +four commits: the ninth session's settled-spans work plus this. Neither needs a +migration and neither touches the schema, so deploying is still `git push` then +`git pull && docker compose up -d --build`. Two things to carry forward. First, +**a hidden Chrome tab fires no `requestAnimationFrame`**, and `recomputeRail` +lives inside one — so the rail freezes under JS-tool automation and every reading +of it is stale until a screenshot forces a paint. Second, **the binary embeds +`web/dist`**: rebuild the frontend and you must rebuild the binary, or the +bundle-hash check will (rightly) fail. Untouched: item 8's keyboard flow, item 3's +incremental half.)* + **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 @@ -974,10 +1085,18 @@ 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 (ninth session onward):** three things remain in the whole -review. **Accept All per category** is the one with real value left — five -tense fixes are still five clicks, and item 2's stable ids make a batch safe to -reason about; the one design question it has to answer is that its undo must be +**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. + +*(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 — +five tense fixes are still five clicks, and item 2's stable ids make a batch safe +to reason about; the one design question it has to answer is that its undo must be a single step. **Keyboard triage** is next, and is bigger than it looks: item 7 made the anchored popover the primary surface in both layouts, so "cycle the underlines" now means driving that popover, not the rail. **Item 3's diff --git a/web/src/App.tsx b/web/src/App.tsx index bf9a289..c100600 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -367,6 +367,30 @@ export default function App() { [removeSuggestion, resolveServerId], ) + // Accept-all: EditorCore has already applied the whole category in one editor + // transaction, so this is only the bookkeeping. Each row is filed individually + // (there's no batch endpoint, and each accept plants its own word in the + // garden), but the kitten cheers once — five cheers for one click would read as + // five separate congratulations for a decision she made once. + const handleAcceptMany = useCallback( + async (list: Suggestion[]) => { + if (list.length === 0) return + for (const s of list) removeSuggestion(s.id) + setAcceptTick((n) => n + 1) + await Promise.all( + list.map(async (s) => { + try { + const id = await resolveServerId(s) + if (id) await api.acceptSuggestion(id) + } catch (err) { + console.error('accept failed', err) + } + }), + ) + }, + [removeSuggestion, resolveServerId], + ) + // After restoring a version, swap the restored doc into the editor. Bumping // editorEpoch remounts EditorCore so it picks up the restored content. const handleRestored = useCallback( @@ -569,6 +593,7 @@ export default function App() { onChange={handleEditorChange} suggestions={suggestions} onAccept={handleAccept} + onAcceptMany={handleAcceptMany} onDismiss={handleDismiss} onVoiceCheck={runVoice} voicing={voicing} diff --git a/web/src/components/Editor/EditorCore.tsx b/web/src/components/Editor/EditorCore.tsx index 04de890..df3c5a5 100644 --- a/web/src/components/Editor/EditorCore.tsx +++ b/web/src/components/Editor/EditorCore.tsx @@ -15,7 +15,7 @@ import TableHeader from '@tiptap/extension-table-header' import TableCell from '@tiptap/extension-table-cell' import { FontSize } from './FontSize' import type { EditorView } from '@tiptap/pm/view' -import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { Toolbar } from '../Toolbar/Toolbar' import { SuggestionCard } from './SuggestionCard' import { SuggestionRail, type RailItem } from './SuggestionRail' @@ -30,7 +30,8 @@ import { SearchHighlight } from './SearchHighlight' import { FindReplace } from './FindReplace' import { Typography } from './Typography' import { RewritePreview, type RewriteStatus } from './RewritePreview' -import { api, type Suggestion, type WordInfo } from '../../api/client' +import { planBatch } from './acceptBatch' +import { api, type Suggestion, type SuggestionType, type WordInfo } from '../../api/client' import { speak, speechSupported } from '../../audio/speech' import type { SpellChecker } from '../../hooks/useSpellChecker' import { usePack } from '../../i18n' @@ -54,6 +55,9 @@ interface Props { // call + state removal (accept's text replacement happens here in the editor). suggestions: Suggestion[] onAccept: (s: Suggestion) => void + // A whole category accepted at once. The text replacement happens here, in one + // transaction; the parent files each row and cheers once for the batch. + onAcceptMany: (list: Suggestion[]) => void onDismiss: (s: Suggestion) => void // Triggers the whole-document voice-consistency pass; `voicing` is true while // it runs (drives the toolbar button's loading state). @@ -221,6 +225,7 @@ export function EditorCore({ onChange, suggestions, onAccept, + onAcceptMany, onDismiss, onVoiceCheck, voicing, @@ -638,18 +643,28 @@ export function EditorCore({ // anchored to the highlight itself (captured before the replacement removes it), // so it fires in the right spot whether the accept came from the hover card or // the margin rail. + // Where a suggestion's highlight sits, in wrapper coordinates — captured before + // the replacement removes it, so the confetti fires over the words that changed. + const burstAt = useCallback((id: string): { top: number; left: number } | null => { + const wrapper = wrapperRef.current + const el = wrapper?.querySelector( + `.petal-suggestion[data-suggestion-id="${CSS.escape(id)}"]`, + ) as HTMLElement | null + if (!wrapper || !el) return null + const wrapRect = wrapper.getBoundingClientRect() + const elRect = el.getBoundingClientRect() + return { top: elRect.top - wrapRect.top, left: elRect.right - wrapRect.left } + }, []) + + const showConfetti = useCallback((burst: { top: number; left: number }) => { + setConfetti(burst) + clearTimeout(confettiTimer.current) + confettiTimer.current = setTimeout(() => setConfetti(null), 720) + }, []) + const handleAccept = useCallback( (s: Suggestion) => { - const wrapper = wrapperRef.current - const el = wrapper?.querySelector( - `.petal-suggestion[data-suggestion-id="${CSS.escape(s.id)}"]`, - ) as HTMLElement | null - let burst: { top: number; left: number } | null = null - if (wrapper && el) { - const wrapRect = wrapper.getBoundingClientRect() - const elRect = el.getBoundingClientRect() - burst = { top: elRect.top - wrapRect.top, left: elRect.right - wrapRect.left } - } + let burst = burstAt(s.id) if (editor && s.replacement.trim() !== '') { const range = findRange(editor.state.doc, s.original) if (range) { @@ -658,16 +673,57 @@ export function EditorCore({ } // Fall back to the hover card's position if the highlight wasn't found. if (!burst && hover) burst = { top: hover.top, left: hover.left + 16 } - if (burst) { - setConfetti(burst) - clearTimeout(confettiTimer.current) - confettiTimer.current = setTimeout(() => setConfetti(null), 720) - } + if (burst) showConfetti(burst) closeCard() setRailExpandedId(null) onAccept(s) }, - [editor, onAccept, closeCard, hover], + [editor, onAccept, closeCard, hover, burstAt, showConfetti], + ) + + // How many pending cards of each type could be accepted in one go. A card + // offers "Accept all" only when it has company, so a lone Grammar card doesn't + // grow a second button saying the same thing as the first. + const batchCounts = useMemo(() => { + const counts: Partial> = {} + for (const s of suggestions) { + if (s.replacement.trim() === '') continue // awareness-only: nothing to accept + counts[s.type] = (counts[s.type] ?? 0) + 1 + } + return counts + }, [suggestions]) + + // Accept a whole category at once. Five tense fixes were five clicks, five + // confetti bursts and five separate undo steps; this is one of each. The single + // undo step is the reason every replacement goes into ONE chain: Tiptap applies + // a chain as a single transaction, and prosemirror-history undoes it as a + // single event, so Ctrl+Z takes back the batch rather than unpicking it. + const handleAcceptAll = useCallback( + (type: SuggestionType) => { + if (!editor) return + const family = suggestions.filter((s) => s.type === type) + const plan = planBatch(family, (original) => findRange(editor.state.doc, original)) + const settled = [...plan.steps.map((step) => step.suggestion), ...plan.missing] + if (settled.length === 0) return + + // The topmost span that's about to change — the last step, since steps run + // bottom-up. Read before the edit, while the highlights still exist. + const first = plan.steps[plan.steps.length - 1] + const burst = first ? burstAt(first.suggestion.id) : null + + if (plan.steps.length > 0) { + let chain = editor.chain().focus() + for (const step of plan.steps) { + chain = chain.insertContentAt({ from: step.from, to: step.to }, step.suggestion.replacement) + } + chain.run() + } + if (burst) showConfetti(burst) + closeCard() + setRailExpandedId(null) + onAcceptMany(settled) + }, + [editor, suggestions, onAcceptMany, closeCard, burstAt, showConfetti], ) const handleDismiss = useCallback( @@ -1248,7 +1304,9 @@ export function EditorCore({ void + onAcceptAll: (type: SuggestionType) => void onDismiss: (s: Suggestion) => void onPointerEnter: () => void onPointerLeave: () => void @@ -30,7 +34,9 @@ interface Props { export function SuggestionCard({ suggestion, style, + batchCount, onAccept, + onAcceptAll, onDismiss, onPointerEnter, onPointerLeave, @@ -144,6 +150,17 @@ export function SuggestionCard({ Dismiss + + {hasReplacement && batchCount > 1 && ( + + )} ) } diff --git a/web/src/components/Editor/SuggestionRail.tsx b/web/src/components/Editor/SuggestionRail.tsx index 242921c..cd5bb31 100644 --- a/web/src/components/Editor/SuggestionRail.tsx +++ b/web/src/components/Editor/SuggestionRail.tsx @@ -1,8 +1,9 @@ import { forwardRef, useLayoutEffect, useRef, useState } from 'react' -import type { Suggestion } from '../../api/client' +import type { Suggestion, SuggestionType } from '../../api/client' import { usePack } from '../../i18n' import { AskPetal } from './AskPetal' -import { TYPE_META, typeLabel } from './suggestionMeta' +import { batchLeaders } from './acceptBatch' +import { TYPE_META, batchLabel, typeLabel } from './suggestionMeta' // Vertical breathing room kept between stacked cards when their natural anchors // would otherwise collide. @@ -22,7 +23,11 @@ interface Props { activeId: string | null // The card expanded to show the full explanation + Ask Petal, or null. expandedId: string | null + // Pending suggestions per type. A card whose type has company offers to take + // the whole category in one step (and one undo step). + batchCounts: Partial> onAccept: (s: Suggestion) => void + onAcceptAll: (type: SuggestionType) => void onDismiss: (s: Suggestion) => void // Pointer entering/leaving a card, so the matching highlight can light up. onHover: (id: string | null) => void @@ -46,7 +51,9 @@ export function SuggestionRail({ items, activeId, expandedId, + batchCounts, onAccept, + onAcceptAll, onDismiss, onHover, onActivate, @@ -93,6 +100,9 @@ export function SuggestionRail({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [layoutKey, expandedId, measureTick]) + // One card per category carries the batch control — see batchLeaders. + const batchLead = batchLeaders(ordered.map(({ suggestion }) => suggestion)) + return (
{ordered.map(({ suggestion }) => ( @@ -112,7 +122,9 @@ export function SuggestionRail({ top={tops[suggestion.id] ?? 0} active={activeId === suggestion.id} expanded={expandedId === suggestion.id} + batchCount={batchLead.has(suggestion.id) ? (batchCounts[suggestion.type] ?? 0) : 0} onAccept={onAccept} + onAcceptAll={onAcceptAll} onDismiss={onDismiss} onHover={onHover} onActivate={onActivate} @@ -128,7 +140,9 @@ interface CardProps { top: number active: boolean expanded: boolean + batchCount: number onAccept: (s: Suggestion) => void + onAcceptAll: (type: SuggestionType) => void onDismiss: (s: Suggestion) => void onHover: (id: string | null) => void onActivate: (id: string) => void @@ -136,7 +150,7 @@ interface CardProps { } const RailCard = forwardRef(function RailCard( - { suggestion, top, active, expanded, onAccept, onDismiss, onHover, onActivate, onToggleExpand }, + { suggestion, top, active, expanded, batchCount, onAccept, onAcceptAll, onDismiss, onHover, onActivate, onToggleExpand }, ref, ) { const pack = usePack() @@ -230,6 +244,17 @@ const RailCard = forwardRef(function RailCard( {expanded ? 'Hide Petal' : 'Ask Petal ✨'}
+ + {hasReplacement && batchCount > 1 && ( + + )} ) }) diff --git a/web/src/components/Editor/acceptBatch.test.ts b/web/src/components/Editor/acceptBatch.test.ts new file mode 100644 index 0000000..dcf3e65 --- /dev/null +++ b/web/src/components/Editor/acceptBatch.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from 'vitest' +import type { Suggestion, SuggestionType } from '../../api/client' +import { batchLeaders, planBatch } from './acceptBatch' + +function sug(id: string, original: string, replacement: string, type: SuggestionType = 'grammar'): Suggestion { + return { + id, + doc_id: 'd1', + from_pos: 0, + to_pos: 0, + original, + replacement, + explanation: '', + type, + status: 'pending', + created_at: '2026-07-28T00:00:00Z', + } +} + +// A stand-in for findRange: first occurrence in a flat string, or null. +function finder(text: string) { + return (needle: string) => { + const i = text.indexOf(needle) + return i < 0 ? null : { from: i, to: i + needle.length } + } +} + +describe('planBatch', () => { + it('applies from the end backwards, so earlier spans keep their positions', () => { + const text = 'a apple and a orange and a egg' + const plan = planBatch( + [sug('1', 'a apple', 'an apple'), sug('2', 'a orange', 'an orange'), sug('3', 'a egg', 'an egg')], + finder(text), + ) + expect(plan.steps.map((s) => s.suggestion.id)).toEqual(['3', '2', '1']) + expect(plan.missing).toEqual([]) + expect(plan.skipped).toEqual([]) + + // Applying the plan in order against a mutable string must land every + // replacement where it belongs — this is the property the ordering exists + // for, and the one a single shared transaction depends on. + let out = text + for (const step of plan.steps) { + out = out.slice(0, step.from) + step.suggestion.replacement + out.slice(step.to) + } + expect(out).toBe('an apple and an orange and an egg') + }) + + it('orders by position in the document, not by the order the cards arrived', () => { + const plan = planBatch( + [sug('late', 'a egg', 'an egg'), sug('early', 'a apple', 'an apple')], + finder('a apple and a egg'), + ) + expect(plan.steps.map((s) => s.suggestion.id)).toEqual(['late', 'early']) + }) + + it('settles a card whose span is gone — she already fixed it herself', () => { + const plan = planBatch( + [sug('1', 'a apple', 'an apple'), sug('2', 'a orange', 'an orange')], + finder('an apple and a orange'), + ) + expect(plan.steps.map((s) => s.suggestion.id)).toEqual(['2']) + expect(plan.missing.map((s) => s.id)).toEqual(['1']) + }) + + it('leaves the second of two cards quoting the same words on screen', () => { + // findRange resolves both to the first occurrence, so applying both would + // overwrite the first edit with the second. The batch takes one and says so. + const plan = planBatch( + [sug('1', 'a apple', 'an apple'), sug('2', 'a apple', 'the apple')], + finder('a apple, a apple'), + ) + expect(plan.steps.map((s) => s.suggestion.id)).toEqual(['1']) + expect(plan.skipped.map((s) => s.id)).toEqual(['2']) + expect(plan.missing).toEqual([]) + }) + + it('keeps both of two spans that merely touch', () => { + // Adjacent is not overlapping: "to" ends exactly where "the" begins. + const plan = planBatch( + [sug('1', 'aa', 'AA'), sug('2', 'bb', 'BB')], + finder('aabb'), + ) + expect(plan.steps.map((s) => s.suggestion.id)).toEqual(['2', '1']) + expect(plan.skipped).toEqual([]) + }) + + it('skips an awareness-only card with nothing to insert', () => { + const plan = planBatch([sug('v', 'a apple', ' ', 'voice')], finder('a apple')) + expect(plan.steps).toEqual([]) + expect(plan.skipped.map((s) => s.id)).toEqual(['v']) + }) + + it('returns an empty plan for an empty family', () => { + const plan = planBatch([], finder('anything')) + expect(plan).toEqual({ steps: [], missing: [], skipped: [] }) + }) +}) + +describe('batchLeaders', () => { + const card = (id: string, type: SuggestionType) => ({ id, type }) + + it('gives each category exactly one leader, the first of its kind', () => { + const leaders = batchLeaders([ + card('g1', 'grammar'), + card('m1', 'mechanics'), + card('g2', 'grammar'), + card('m2', 'mechanics'), + card('g3', 'grammar'), + ]) + expect([...leaders]).toEqual(['g1', 'm1']) + }) + + it('follows the stacking order it is given, not the order types appear elsewhere', () => { + // The rail sorts by anchor, so "first" means highest on the page — the card + // she reads first, which is where the batch control belongs. + const leaders = batchLeaders([card('lower', 'grammar'), card('upper', 'grammar')]) + expect([...leaders]).toEqual(['lower']) + }) + + it('leads a lone card of its type too — the count is what hides the button', () => { + expect([...batchLeaders([card('only', 'voice')])]).toEqual(['only']) + }) + + it('has no leaders for an empty stack', () => { + expect(batchLeaders([]).size).toBe(0) + }) +}) diff --git a/web/src/components/Editor/acceptBatch.ts b/web/src/components/Editor/acceptBatch.ts new file mode 100644 index 0000000..b5abdd8 --- /dev/null +++ b/web/src/components/Editor/acceptBatch.ts @@ -0,0 +1,84 @@ +import type { Suggestion, SuggestionType } from '../../api/client' + +// batchLeaders picks the one card per category that carries the "Accept all +// Grammar (5)" control, given the cards in the order they are stacked. The rail +// can't carry a category header — cards are anchored to their own sentence, so a +// type's cards are scattered down the column — and the closest honest stand-in +// is the first card of its kind. Offering the same batch on all five would be +// five buttons saying one thing. +export function batchLeaders(ordered: { type: SuggestionType; id: string }[]): Set { + const leaders = new Set() + const claimed = new Set() + for (const { type, id } of ordered) { + if (claimed.has(type)) continue + claimed.add(type) + leaders.add(id) + } + return leaders +} + +export interface BatchStep { + suggestion: Suggestion + from: number + to: number +} + +export interface BatchPlan { + // The edits to apply, ordered LAST span first. Every position is resolved + // against the document as it stands *before* any of them are applied, and + // applying from the end backwards means an earlier span's position can't be + // shifted by a later one — so the whole batch can go into a single + // transaction, which is the point: one Ctrl+Z puts it all back. + steps: BatchStep[] + // Cards whose span is no longer in the document — she fixed it herself, or + // edited around it since the check ran. There is nothing to replace, but the + // card is stale and settling it is what a single Accept already does. + missing: Suggestion[] + // Cards deliberately left on screen: two suggestions quoting the same words + // (findRange resolves both to the first occurrence, so applying the second + // would overwrite the first), and anything with nothing to insert. A batch + // that silently dropped these would report edits it never made. + skipped: Suggestion[] +} + +// planBatch decides what one "Accept all " click actually does. It is +// pure and takes the span resolver as an argument so the ordering and overlap +// rules can be tested without a ProseMirror document — the part that goes wrong +// is the arithmetic, not the lookup. +export function planBatch( + list: Suggestion[], + resolve: (original: string) => { from: number; to: number } | null, +): BatchPlan { + const located: BatchStep[] = [] + const missing: Suggestion[] = [] + const skipped: Suggestion[] = [] + + for (const suggestion of list) { + if (suggestion.replacement.trim() === '') { + skipped.push(suggestion) // awareness-only (voice): nothing to accept + continue + } + const range = resolve(suggestion.original) + if (!range) { + missing.push(suggestion) + continue + } + located.push({ suggestion, from: range.from, to: range.to }) + } + + // Document order, then greedily keep the non-overlapping ones. + located.sort((a, b) => a.from - b.from || a.to - b.to) + const steps: BatchStep[] = [] + let cursor = -Infinity + for (const step of located) { + if (step.from < cursor) { + skipped.push(step.suggestion) + continue + } + steps.push(step) + cursor = step.to + } + steps.reverse() + + return { steps, missing, skipped } +} diff --git a/web/src/components/Editor/suggestionMeta.ts b/web/src/components/Editor/suggestionMeta.ts index 0ab9aea..b0ec848 100644 --- a/web/src/components/Editor/suggestionMeta.ts +++ b/web/src/components/Editor/suggestionMeta.ts @@ -28,3 +28,13 @@ export const TYPE_META: Record export function typeLabel(type: SuggestionType, pack: Pack): string { return type === 'translate' ? pack.editor.translateLabel : TYPE_META[type].label } + +// The label on the accept-a-whole-category button. It stays English even on a +// translation card, whose pill is bilingual: the pill names the kind of advice +// she is reading, while this names an action taken over the rest of the queue, +// and every other word in the action row — Accept, Dismiss, Ask Petal — is +// English too. A control that switched languages between cards would read as a +// different control. +export function batchLabel(type: SuggestionType, count: number): string { + return `Accept all ${TYPE_META[type].label} (${count})` +} diff --git a/web/src/index.css b/web/src/index.css index 70124be..c26044e 100644 --- a/web/src/index.css +++ b/web/src/index.css @@ -314,6 +314,19 @@ button, a, input { color: var(--color-plum); } +/* Accept-all: the quieter sibling of Accept, on both card surfaces. It's outlined + in its category's own colour rather than filled like Accept, because it acts on + cards she can't see from here — an equally loud button for a larger action would + invite the click she meant to give the one suggestion in front of her. */ +.petal-accept-all { + border: 1px solid; + background: transparent; + transition: background-color 120ms ease; +} +.petal-accept-all:hover { + background: var(--color-surface-alt); +} + /* --- Find & Replace --------------------------------------------------------- In-document search (Ctrl/Cmd+F). Every match gets a soft honey wash; the current match is brighter with a rose ring so it stands out as you step