Merge feat/accept-all-category: a whole category in one click, and one undo
This commit is contained in:
+123
-4
@@ -861,6 +861,105 @@ one from many.
|
|||||||
**Still open in item 8:** Accept All per category, and the keyboard triage
|
**Still open in item 8:** Accept All per category, and the keyboard triage
|
||||||
flow. Both untouched.
|
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)
|
## 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
|
different cause, and the same bundle-hash check catches it. Untouched: item
|
||||||
8's Accept All and keyboard flow, item 3's incremental half.)*
|
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
|
**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
|
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
|
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
|
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).
|
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
|
**Suggested next (tenth session onward):** two things remain in the whole review.
|
||||||
review. **Accept All per category** is the one with real value left — five
|
**Keyboard triage** is the one to take: it is the last of item 8, and Accept All
|
||||||
tense fixes are still five clicks, and item 2's stable ids make a batch safe to
|
just built half of what it needs — a category is now a thing the UI can act on in
|
||||||
reason about; the one design question it has to answer is that its undo must be
|
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
|
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
|
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
|
underlines" now means driving that popover, not the rail. **Item 3's
|
||||||
|
|||||||
@@ -367,6 +367,30 @@ export default function App() {
|
|||||||
[removeSuggestion, resolveServerId],
|
[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
|
// After restoring a version, swap the restored doc into the editor. Bumping
|
||||||
// editorEpoch remounts EditorCore so it picks up the restored content.
|
// editorEpoch remounts EditorCore so it picks up the restored content.
|
||||||
const handleRestored = useCallback(
|
const handleRestored = useCallback(
|
||||||
@@ -569,6 +593,7 @@ export default function App() {
|
|||||||
onChange={handleEditorChange}
|
onChange={handleEditorChange}
|
||||||
suggestions={suggestions}
|
suggestions={suggestions}
|
||||||
onAccept={handleAccept}
|
onAccept={handleAccept}
|
||||||
|
onAcceptMany={handleAcceptMany}
|
||||||
onDismiss={handleDismiss}
|
onDismiss={handleDismiss}
|
||||||
onVoiceCheck={runVoice}
|
onVoiceCheck={runVoice}
|
||||||
voicing={voicing}
|
voicing={voicing}
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ import TableHeader from '@tiptap/extension-table-header'
|
|||||||
import TableCell from '@tiptap/extension-table-cell'
|
import TableCell from '@tiptap/extension-table-cell'
|
||||||
import { FontSize } from './FontSize'
|
import { FontSize } from './FontSize'
|
||||||
import type { EditorView } from '@tiptap/pm/view'
|
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 { Toolbar } from '../Toolbar/Toolbar'
|
||||||
import { SuggestionCard } from './SuggestionCard'
|
import { SuggestionCard } from './SuggestionCard'
|
||||||
import { SuggestionRail, type RailItem } from './SuggestionRail'
|
import { SuggestionRail, type RailItem } from './SuggestionRail'
|
||||||
@@ -30,7 +30,8 @@ import { SearchHighlight } from './SearchHighlight'
|
|||||||
import { FindReplace } from './FindReplace'
|
import { FindReplace } from './FindReplace'
|
||||||
import { Typography } from './Typography'
|
import { Typography } from './Typography'
|
||||||
import { RewritePreview, type RewriteStatus } from './RewritePreview'
|
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 { speak, speechSupported } from '../../audio/speech'
|
||||||
import type { SpellChecker } from '../../hooks/useSpellChecker'
|
import type { SpellChecker } from '../../hooks/useSpellChecker'
|
||||||
import { usePack } from '../../i18n'
|
import { usePack } from '../../i18n'
|
||||||
@@ -54,6 +55,9 @@ interface Props {
|
|||||||
// call + state removal (accept's text replacement happens here in the editor).
|
// call + state removal (accept's text replacement happens here in the editor).
|
||||||
suggestions: Suggestion[]
|
suggestions: Suggestion[]
|
||||||
onAccept: (s: Suggestion) => void
|
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
|
onDismiss: (s: Suggestion) => void
|
||||||
// Triggers the whole-document voice-consistency pass; `voicing` is true while
|
// Triggers the whole-document voice-consistency pass; `voicing` is true while
|
||||||
// it runs (drives the toolbar button's loading state).
|
// it runs (drives the toolbar button's loading state).
|
||||||
@@ -221,6 +225,7 @@ export function EditorCore({
|
|||||||
onChange,
|
onChange,
|
||||||
suggestions,
|
suggestions,
|
||||||
onAccept,
|
onAccept,
|
||||||
|
onAcceptMany,
|
||||||
onDismiss,
|
onDismiss,
|
||||||
onVoiceCheck,
|
onVoiceCheck,
|
||||||
voicing,
|
voicing,
|
||||||
@@ -638,18 +643,28 @@ export function EditorCore({
|
|||||||
// anchored to the highlight itself (captured before the replacement removes it),
|
// 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
|
// so it fires in the right spot whether the accept came from the hover card or
|
||||||
// the margin rail.
|
// 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(
|
const handleAccept = useCallback(
|
||||||
(s: Suggestion) => {
|
(s: Suggestion) => {
|
||||||
const wrapper = wrapperRef.current
|
let burst = burstAt(s.id)
|
||||||
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 }
|
|
||||||
}
|
|
||||||
if (editor && s.replacement.trim() !== '') {
|
if (editor && s.replacement.trim() !== '') {
|
||||||
const range = findRange(editor.state.doc, s.original)
|
const range = findRange(editor.state.doc, s.original)
|
||||||
if (range) {
|
if (range) {
|
||||||
@@ -658,16 +673,57 @@ export function EditorCore({
|
|||||||
}
|
}
|
||||||
// Fall back to the hover card's position if the highlight wasn't found.
|
// 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 && hover) burst = { top: hover.top, left: hover.left + 16 }
|
||||||
if (burst) {
|
if (burst) showConfetti(burst)
|
||||||
setConfetti(burst)
|
|
||||||
clearTimeout(confettiTimer.current)
|
|
||||||
confettiTimer.current = setTimeout(() => setConfetti(null), 720)
|
|
||||||
}
|
|
||||||
closeCard()
|
closeCard()
|
||||||
setRailExpandedId(null)
|
setRailExpandedId(null)
|
||||||
onAccept(s)
|
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<Record<SuggestionType, number>> = {}
|
||||||
|
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(
|
const handleDismiss = useCallback(
|
||||||
@@ -1248,7 +1304,9 @@ export function EditorCore({
|
|||||||
<SuggestionCard
|
<SuggestionCard
|
||||||
suggestion={hover.suggestion}
|
suggestion={hover.suggestion}
|
||||||
style={{ top: hover.top, left: hover.left }}
|
style={{ top: hover.top, left: hover.left }}
|
||||||
|
batchCount={batchCounts[hover.suggestion.type] ?? 0}
|
||||||
onAccept={handleAccept}
|
onAccept={handleAccept}
|
||||||
|
onAcceptAll={handleAcceptAll}
|
||||||
onDismiss={handleDismiss}
|
onDismiss={handleDismiss}
|
||||||
onPointerEnter={keepOpen}
|
onPointerEnter={keepOpen}
|
||||||
onPointerLeave={scheduleClose}
|
onPointerLeave={scheduleClose}
|
||||||
@@ -1261,7 +1319,9 @@ export function EditorCore({
|
|||||||
items={railItems}
|
items={railItems}
|
||||||
activeId={activeId}
|
activeId={activeId}
|
||||||
expandedId={railExpandedId}
|
expandedId={railExpandedId}
|
||||||
|
batchCounts={batchCounts}
|
||||||
onAccept={handleAccept}
|
onAccept={handleAccept}
|
||||||
|
onAcceptAll={handleAcceptAll}
|
||||||
onDismiss={handleDismiss}
|
onDismiss={handleDismiss}
|
||||||
onHover={setActiveId}
|
onHover={setActiveId}
|
||||||
onActivate={activateRailCard}
|
onActivate={activateRailCard}
|
||||||
|
|||||||
@@ -1,13 +1,17 @@
|
|||||||
import { useEffect, useRef, useState } from 'react'
|
import { useEffect, useRef, useState } from 'react'
|
||||||
import type { Suggestion } from '../../api/client'
|
import type { Suggestion, SuggestionType } from '../../api/client'
|
||||||
import { usePack } from '../../i18n'
|
import { usePack } from '../../i18n'
|
||||||
import { AskPetal } from './AskPetal'
|
import { AskPetal } from './AskPetal'
|
||||||
import { TYPE_META, typeLabel } from './suggestionMeta'
|
import { TYPE_META, batchLabel, typeLabel } from './suggestionMeta'
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
suggestion: Suggestion
|
suggestion: Suggestion
|
||||||
style: React.CSSProperties
|
style: React.CSSProperties
|
||||||
|
// How many pending suggestions share this card's type (including this one).
|
||||||
|
// Two or more offers to take the whole category in one step.
|
||||||
|
batchCount: number
|
||||||
onAccept: (s: Suggestion) => void
|
onAccept: (s: Suggestion) => void
|
||||||
|
onAcceptAll: (type: SuggestionType) => void
|
||||||
onDismiss: (s: Suggestion) => void
|
onDismiss: (s: Suggestion) => void
|
||||||
onPointerEnter: () => void
|
onPointerEnter: () => void
|
||||||
onPointerLeave: () => void
|
onPointerLeave: () => void
|
||||||
@@ -30,7 +34,9 @@ interface Props {
|
|||||||
export function SuggestionCard({
|
export function SuggestionCard({
|
||||||
suggestion,
|
suggestion,
|
||||||
style,
|
style,
|
||||||
|
batchCount,
|
||||||
onAccept,
|
onAccept,
|
||||||
|
onAcceptAll,
|
||||||
onDismiss,
|
onDismiss,
|
||||||
onPointerEnter,
|
onPointerEnter,
|
||||||
onPointerLeave,
|
onPointerLeave,
|
||||||
@@ -144,6 +150,17 @@ export function SuggestionCard({
|
|||||||
Dismiss
|
Dismiss
|
||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{hasReplacement && batchCount > 1 && (
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() => onAcceptAll(suggestion.type)}
|
||||||
|
className="petal-accept-all mt-2 w-full rounded-full py-1.5 text-xs font-bold"
|
||||||
|
style={{ color: 'var(--color-plum)', borderColor: meta.color }}
|
||||||
|
>
|
||||||
|
{batchLabel(suggestion.type, batchCount)}
|
||||||
|
</button>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,8 +1,9 @@
|
|||||||
import { forwardRef, useLayoutEffect, useRef, useState } from 'react'
|
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 { usePack } from '../../i18n'
|
||||||
import { AskPetal } from './AskPetal'
|
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
|
// Vertical breathing room kept between stacked cards when their natural anchors
|
||||||
// would otherwise collide.
|
// would otherwise collide.
|
||||||
@@ -22,7 +23,11 @@ interface Props {
|
|||||||
activeId: string | null
|
activeId: string | null
|
||||||
// The card expanded to show the full explanation + Ask Petal, or null.
|
// The card expanded to show the full explanation + Ask Petal, or null.
|
||||||
expandedId: string | 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<Record<SuggestionType, number>>
|
||||||
onAccept: (s: Suggestion) => void
|
onAccept: (s: Suggestion) => void
|
||||||
|
onAcceptAll: (type: SuggestionType) => void
|
||||||
onDismiss: (s: Suggestion) => void
|
onDismiss: (s: Suggestion) => void
|
||||||
// Pointer entering/leaving a card, so the matching highlight can light up.
|
// Pointer entering/leaving a card, so the matching highlight can light up.
|
||||||
onHover: (id: string | null) => void
|
onHover: (id: string | null) => void
|
||||||
@@ -46,7 +51,9 @@ export function SuggestionRail({
|
|||||||
items,
|
items,
|
||||||
activeId,
|
activeId,
|
||||||
expandedId,
|
expandedId,
|
||||||
|
batchCounts,
|
||||||
onAccept,
|
onAccept,
|
||||||
|
onAcceptAll,
|
||||||
onDismiss,
|
onDismiss,
|
||||||
onHover,
|
onHover,
|
||||||
onActivate,
|
onActivate,
|
||||||
@@ -93,6 +100,9 @@ export function SuggestionRail({
|
|||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [layoutKey, expandedId, measureTick])
|
}, [layoutKey, expandedId, measureTick])
|
||||||
|
|
||||||
|
// One card per category carries the batch control — see batchLeaders.
|
||||||
|
const batchLead = batchLeaders(ordered.map(({ suggestion }) => suggestion))
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="petal-rail petal-no-print" aria-label="Suggestions">
|
<div className="petal-rail petal-no-print" aria-label="Suggestions">
|
||||||
{ordered.map(({ suggestion }) => (
|
{ordered.map(({ suggestion }) => (
|
||||||
@@ -112,7 +122,9 @@ export function SuggestionRail({
|
|||||||
top={tops[suggestion.id] ?? 0}
|
top={tops[suggestion.id] ?? 0}
|
||||||
active={activeId === suggestion.id}
|
active={activeId === suggestion.id}
|
||||||
expanded={expandedId === suggestion.id}
|
expanded={expandedId === suggestion.id}
|
||||||
|
batchCount={batchLead.has(suggestion.id) ? (batchCounts[suggestion.type] ?? 0) : 0}
|
||||||
onAccept={onAccept}
|
onAccept={onAccept}
|
||||||
|
onAcceptAll={onAcceptAll}
|
||||||
onDismiss={onDismiss}
|
onDismiss={onDismiss}
|
||||||
onHover={onHover}
|
onHover={onHover}
|
||||||
onActivate={onActivate}
|
onActivate={onActivate}
|
||||||
@@ -128,7 +140,9 @@ interface CardProps {
|
|||||||
top: number
|
top: number
|
||||||
active: boolean
|
active: boolean
|
||||||
expanded: boolean
|
expanded: boolean
|
||||||
|
batchCount: number
|
||||||
onAccept: (s: Suggestion) => void
|
onAccept: (s: Suggestion) => void
|
||||||
|
onAcceptAll: (type: SuggestionType) => void
|
||||||
onDismiss: (s: Suggestion) => void
|
onDismiss: (s: Suggestion) => void
|
||||||
onHover: (id: string | null) => void
|
onHover: (id: string | null) => void
|
||||||
onActivate: (id: string) => void
|
onActivate: (id: string) => void
|
||||||
@@ -136,7 +150,7 @@ interface CardProps {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const RailCard = forwardRef<HTMLDivElement, CardProps>(function RailCard(
|
const RailCard = forwardRef<HTMLDivElement, CardProps>(function RailCard(
|
||||||
{ suggestion, top, active, expanded, onAccept, onDismiss, onHover, onActivate, onToggleExpand },
|
{ suggestion, top, active, expanded, batchCount, onAccept, onAcceptAll, onDismiss, onHover, onActivate, onToggleExpand },
|
||||||
ref,
|
ref,
|
||||||
) {
|
) {
|
||||||
const pack = usePack()
|
const pack = usePack()
|
||||||
@@ -230,6 +244,17 @@ const RailCard = forwardRef<HTMLDivElement, CardProps>(function RailCard(
|
|||||||
{expanded ? 'Hide Petal' : 'Ask Petal ✨'}
|
{expanded ? 'Hide Petal' : 'Ask Petal ✨'}
|
||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{hasReplacement && batchCount > 1 && (
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() => onAcceptAll(suggestion.type)}
|
||||||
|
className="petal-accept-all mt-2 w-full rounded-full py-1 text-[0.7rem] font-bold"
|
||||||
|
style={{ color: 'var(--color-plum)', borderColor: meta.color }}
|
||||||
|
>
|
||||||
|
{batchLabel(suggestion.type, batchCount)}
|
||||||
|
</button>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -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)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -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<string> {
|
||||||
|
const leaders = new Set<string>()
|
||||||
|
const claimed = new Set<SuggestionType>()
|
||||||
|
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 <category>" 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 }
|
||||||
|
}
|
||||||
@@ -28,3 +28,13 @@ export const TYPE_META: Record<SuggestionType, { color: string; label: string }>
|
|||||||
export function typeLabel(type: SuggestionType, pack: Pack): string {
|
export function typeLabel(type: SuggestionType, pack: Pack): string {
|
||||||
return type === 'translate' ? pack.editor.translateLabel : TYPE_META[type].label
|
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})`
|
||||||
|
}
|
||||||
|
|||||||
@@ -314,6 +314,19 @@ button, a, input {
|
|||||||
color: var(--color-plum);
|
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 ---------------------------------------------------------
|
/* --- Find & Replace ---------------------------------------------------------
|
||||||
In-document search (Ctrl/Cmd+F). Every match gets a soft honey wash; the
|
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
|
current match is brighter with a rose ring so it stands out as you step
|
||||||
|
|||||||
Reference in New Issue
Block a user