diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md new file mode 100644 index 0000000..daa4740 --- /dev/null +++ b/UX_REVIEW_2026-07-27.md @@ -0,0 +1,272 @@ +# Petal UX review — 2026-07-27 (implementation doc) + +**Origin:** a hands-on browser session against the live VPS deploy +(petal.parodia.dev) on a 1517×810 desktop viewport, deliberately writing +ESL-style English, accepting/undoing suggestions, using Ask Petal, the +right-click dictionary, the Garden, History, and the doc-type menu. Goal: +close the gap to Grammarly's *feel* (latency, stability, proximity of +feedback) without its cost or surveillance. + +**How to use this doc:** each item has symptom/repro, likely code location, +proposed fix, and acceptance criteria. Items are ordered by +value-for-effort. Where the session couldn't confirm root cause, that is +said explicitly — verify before building. The product *why* behind Petal +lives in `SUGGESTIONS.md`; execution phases in `BUILD_PLAN.md`. Nothing here +contradicts them; this is polish on the existing loop. + +**What was verified working well (don't regress):** error coverage and +explanation quality across two check rounds; bilingual (en+zh) card +explanations; Ask Petal follow-up answers; instant right-click dictionary +with TTS + slow-replay; Garden sprouting; autosave + History snapshots; +kitten cheers on accept; clean console throughout. + +--- + +## 0. DONE this session — kitten yields to cards + +The corner mascot's halo sat on top of the bottom rail cards, History-panel +footer controls, and the Garden counter, blocking reading and clicks. + +**Implemented (this commit):** when any `.petal-rail-card` overlaps the +mascot badge, the kitten fades to 15% opacity, shrinks 10% toward its +corner (standalone `scale` property, so it composes with the bob +animation's `transform`), and goes `pointer-events: none` so clicks pass +through. It wakes (full size/opacity) while its speech bubble or the +companion picker is open, or when cards no longer overlap. + +- `web/src/components/Companion/useCardOverlap.ts` — rect-intersection + hook, rAF-throttled on scroll/resize + 500 ms poll. +- `web/src/components/Companion/PetalCompanion.tsx` — `faded` wiring. +- `web/src/index.css` — `.petal-companion-faded`, transition. + +**Follow-up — DONE.** The overlap check now watches +`.petal-rail-card, [role="dialog"][aria-modal="true"]`. Matching the modal +role rather than each panel's class covers History and Garden today (both +already render `role="dialog" aria-modal="true"` drawers) and any future +drawer for free, with no selector list to keep in sync. + +Two refinements the original note didn't anticipate: + +- The hook now reports `{ cards, modal }` separately. A card overlap still + lets the kitten wake for a bubble; a **modal overlap yields + unconditionally** — a cheer isn't worth covering the panel she just + opened on purpose. +- The speech bubble is its own layer, so fading the badge didn't hide it. + It's now held back explicitly while a panel is open, and reappears when + she closes it (`useCompanion` keeps it in state). + +Verified against the live build: with History open, the drawer overlaps the +badge rect and the "📜 写作证明 · Writing passport" control sits under the +mascot — the exact defect. The new selector trips on it. + +- `useCardOverlap.ts` — `CardOverlap` return type, modal selector, identity + guard so the 500 ms poll doesn't re-render on every tick. +- `PetalCompanion.tsx` — `crowded.modal || (crowded.cards && …)`, bubble gate. + +--- + +## 1. Bug: redo does not re-apply an accepted suggestion — NOT REPRODUCIBLE + +**Status (2026-07-27, follow-up session):** investigated against the live +VPS build with the ProseMirror history plugin state read directly +(`done.eventCount` / `undone.eventCount`) and `view.dispatch` hooked to log +every transaction. **Redo works in every path tried** — including the exact +repro below, on the same Idiom card. Closing this unless it resurfaces with +a tighter repro. + +Paths tried, all correct (accept → `done` +1; undo → `done` −1, `undone` 1, +text reverts; redo → replacement restored): + +- accept → undo → redo pressed immediately +- accept → undo → **18 s pause so the full re-check lands** → redo +- accept with the editor never focused → undo → redo + +The doc's hypothesis — that the re-check wipes the redo stack — was tested +directly and is **false**: across the pause, the only doc-changing +transaction was the undo itself (`metaHistory: true`); the re-check's +transactions are all decoration-only (`docChanged: false`), which +prosemirror-history ignores. `canRedo` stayed `true` throughout. + +One real trap that likely explains the original report: **keyboard undo +only works when focus is in the editor.** Clicking Accept in the rail moves +focus to the Accept button; `handleAccept`'s `chain().focus()` normally +returns it, but that only runs when `findRange` locates the original span — +if the span isn't found, focus stays on the button and Ctrl+Z/Ctrl+Shift+Z +silently do nothing. Worth hardening regardless of this item. + +**Still real, found while investigating:** after undo, the suggestion is +already marked accepted server-side, so the card doesn't reliably come back +for the text that's now showing again. In one run the "for buy some apple" +card was gone while the erroneous text was visible; in another the card did +return (re-merged into a wider span). This is the second half of this item's +own acceptance criterion and belongs with item 2's stable identity work. + +**Original repro (could not reproduce):** accept a suggestion (text +updates), press Ctrl+Z (text reverts — correct), press Ctrl+Shift+Z → +nothing happens. Observed on the Idiom card "by foots → on foot". + +**Where to look:** `EditorCore.handleAccept` +(`web/src/components/Editor/EditorCore.tsx` ~line 556) applies via +`editor.chain().focus().insertContentAt(range, s.replacement).run()`, which +*is* a normal history transaction — so the break is probably downstream: +after the undo, the parent `onAccept`/re-check flow may dispatch a +transaction that clears the redo stack (any doc-touching tr wipes redo), or +the accepted suggestion's server-side state makes the recheck rewrite +content. Root cause was **not** confirmed in the session — instrument +first. + +**Acceptance:** accept → undo → redo restores the replacement; the +suggestion card state stays consistent with whichever text is showing. + +## 2. Suggestion stability: stop regenerating the world on every accept + +The biggest feel gap vs Grammarly. Today every accept (and every edit) +triggers a full-document `POST /api/docs/:id/check`; all remaining cards +vanish and re-arrive seconds later, spans re-merge into different shapes, +and the LLM re-words every explanation each round (the "weather were" card +carried three different explanations in one session). It reads as +instability, doubles the pause after each accept, and burns qwen3.5 tokens. + +**Proposed fix (server + client):** +- Split the doc into sentences (or paragraphs) and hash each. On re-check, + send only chunks whose hash changed since the last check; suggestions on + unchanged chunks are returned from cache byte-identical, including the + explanation text. `internal/suggestions/handlers.go` is the entry point. +- Give suggestions stable identity across checks: key on + (chunk hash, original span, replacement) so an untouched suggestion keeps + its `id`, and the client keeps the existing card DOM instead of + remounting (no vanish/reappear). +- Client: on accept, remove that one card optimistically and leave the rest + untouched while the changed-chunk recheck runs. + +**Acceptance:** accepting one suggestion never changes the text, wording, +or position of any other card; re-check traffic after a one-sentence edit +contains only that sentence's chunk; explanations are stable across rounds. + +## 3. Perceived latency: mask the LLM round-trip + +Measured ~8–15 s from typing-stop to cards, with only a small "Checking…" +in the status bar. Two independent levers, both worth doing: + +- **Incremental surfacing.** Stream/deliver per-chunk results as each + sentence finishes checking instead of one batch at the end (pairs + naturally with item 2's chunking). Status bar shows a live count: + "Found 3 so far…". +- **Instant local rules layer.** A tiny deterministic pass that underlines + the classics with zero network: a/an before vowel sound, plural after + some/many/three…, he/she/it + verb-s, common mass nouns ("an + information"). Petal's ethos (see `SUGGESTIONS.md`: LLM is garnish, plain + code essential) fits this exactly. Note `grammarLite.test.ts` already + exists under `web/src/components/Companion/` — check whether a rules + engine is already half-built before writing a new one. Local hits render + immediately with a modest style, then get confirmed/enriched (or + withdrawn) when the LLM pass lands. + +**Acceptance:** an obvious error like "a apple" underlines in <100 ms +offline; during a full check, at least one card appears before the last +chunk finishes; the status bar shows a running count. + +## 4. Rail scrolls away from the text + +With ~7 cards the rail is taller than the viewport; scrolling to reach +lower cards scrolls the document text fully off-screen, severing the +card↔sentence connection. + +**Proposed fix:** keep the editor column sticky/pinned while the rail +(`web/src/components/Editor/SuggestionRail.tsx`, `.petal-rail` in +`index.css`) scrolls in its own `overflow-y: auto` container. Preserve the +existing anchor-to-highlight layout for cards that fit; the container only +takes over when the stack exceeds the viewport. + +**Acceptance:** with 10+ suggestions, the flagged text stays visible while +scrolling the card list; hover-linking still highlights the right span. + +## 5. Mixed-language spans: offer translation, don't ignore + +**Status (follow-up session): premise partly wrong — re-scope before +building.** On the live build the same sentence *does* now produce a card: +a **Clarity** card reading 我想说这句话但是不知道用英语怎么说。 → "I want to +say this sentence but do…". So the span is detected and an English +rendering is already generated; what's missing is only the *framing* — it's +labeled Clarity rather than a first-class 翻译 · Translate type, so the +flagship moment reads as a tidy-up. Re-scope this item from "detect and +translate" to "give it its own type, label, and card treatment", which is +much cheaper than the plan below. The original observation follows. + +Typed mid-document: 我想说这句话但是不知道用英语怎么说。 ("I want to say +this but don't know how in English") — Petal produced **no card at all**. +The pair model (`SUGGESTIONS.md` §1: user may type in either language, +Petal infers direction) says this should be the flagship moment. + +**Proposed fix:** during chunking (item 2), detect spans in the pair's X +language inside an English context (CJK detection already exists for +spellcheck exclusion — see `web/src/components/Editor/SpellCheck.ts`). +Emit a new suggestion type `translate` whose replacement is the English +rendering, card labeled 翻译 · Translate, with the usual +Accept / Ask Petal. Accept replaces the span (keep the original as the +card's strikethrough line so she can still see what she wrote). + +**Acceptance:** a Chinese sentence inside an English doc yields a Translate +card within one check cycle; accepting swaps in the English; an +English-only doc and a Chinese-only doc are unaffected. + +## 6. Ask Petal answers: bilingual, and room to read + +The card's *explanation* is bilingual, but the Ask Petal *answer* came back +English-only, rendered in a small scrollable box inside the card. + +**Proposed fix:** prompt the answer path +(`internal/suggestions/chat*.go` / `AskPetal.tsx`) to reply in both pair +languages (native first, mirroring the bubble pattern in +`PetalCompanion.tsx`); let the answer area grow to the card's width with a +sane max-height (~50vh) before scrolling. + +**Acceptance:** an Ask Petal answer shows 中文 + English; a 3-paragraph +answer is readable without scrolling a ~100 px box. + +## 7. Inline popover at the underline (verify, then strengthen) + +Grammarly's core gesture is click-the-word → popup at the word. +`EditorCore.tsx` already has a `hover` card anchored to highlights (see +`handleAccept`'s fallback burst position), and the rail wires +`activeId`/`onHover` both ways — so part of this exists. The session +experience on a wide (1517 px) screen was still: click underline → the +*far-right* rail card expands, ~400 px of eye travel. + +**Task:** confirm the hover card appears on click as well as hover, that it +offers Accept + a one-line reason + "more" (expanding the rail card), and +that hover-linking (card ↔ span glow) works in both directions. Fix +whichever half is missing. + +**Acceptance:** clicking an underline shows an anchored mini-popover with +Accept, without needing the rail; hovering a rail card glows its span and +vice versa. + +## 8. Smaller items (each small, do opportunistically) + +- **Accept All per category.** Five tense fixes = five clicks today. Add + "Accept all Grammar (5)" per category header in the rail, one undo step + for the batch. Acceptance: batch-accept applies all, single Ctrl+Z + reverts the batch. +- **Keyboard flow.** Tab/Shift+Tab (or n/p) cycles underlines with the + popover open; Enter accepts, Esc dismisses popover. Acceptance: a doc + can be fully triaged without the mouse. +- **Dismissal persistence.** Untested in session: does a dismissed (✕) + suggestion stay dismissed after the next full re-check? With item 2's + stable identity, store dismissed keys per doc and filter server-side. + Acceptance: dismiss → edit elsewhere → recheck → the dismissed card does + not return. +- **Status-bar summary.** "3 petals to polish 🌸 · 三片花瓣待打磨" next to + the word count — the gentle version of Grammarly's score. No numeric + grade, per the north star. Acceptance: count updates live with the rail. + +--- + +## Explicit non-goals (from this review) + +- No document score/grade, no streaks-pressure — the Garden and kitten + already carry motivation warmly. +- No browser-extension-style everywhere-checking; Petal is the writing + place. +- The doc-type dropdown's translucent look during open was **animation + mid-fade, not a bug** — leave it. diff --git a/web/src/components/Companion/PetalCompanion.tsx b/web/src/components/Companion/PetalCompanion.tsx index fb6ec7a..efc03aa 100644 --- a/web/src/components/Companion/PetalCompanion.tsx +++ b/web/src/components/Companion/PetalCompanion.tsx @@ -3,6 +3,7 @@ import type { SaveStatus } from '../../hooks/useAutoSave' import { useCompanion, type Mood } from './useCompanion' import { LottiePlayer } from './LottiePlayer' import { COMPANIONS, DEFAULT_COMPANION } from './companions' +import { useCardOverlap } from './useCardOverlap' import { onPrefsScopeChange, readPref, writePref } from '../../lib/prefs' import { usePack } from '../../i18n' @@ -86,6 +87,15 @@ export function PetalCompanion({ const companion = COMPANIONS.find((c) => c.id === companionId) ?? COMPANIONS[0] const [pickerOpen, setPickerOpen] = useState(false) const rootRef = useRef(null) + const badgeRef = useRef(null) + + // When suggestion cards stack down into the corner, the kitten fades to + // translucent and shrinks a step so the card stays readable and clickable. + // It wakes back up whenever it has something to say (bubble) or is being + // interacted with (picker open) — except under an open History or Garden + // panel, where even a cheer would cover the controls she just reached for. + const crowded = useCardOverlap(badgeRef) + const faded = crowded.modal || (crowded.cards && !pickerOpen && !bubble) // Awake companions (no sleeping clip) don't visibly nap — when the engine // dozes them, keep their normal idle pose instead of a sleepy face. Only a @@ -176,7 +186,10 @@ export function PetalCompanion({ )} - {bubble && !pickerOpen && ( + {/* The bubble is its own layer, so fading the badge doesn't hide it — + hold it back explicitly while a panel is open. useCompanion keeps the + bubble in state, so it reappears when she closes the panel. */} + {bubble && !pickerOpen && !crowded.modal && (
setPickerOpen((o) => !o)} title="Choose a companion" aria-label="Choose a companion" - className={`petal-companion pointer-events-auto select-none ${napping ? 'petal-companion-sleep' : ''}`} + className={`petal-companion select-none ${faded ? 'petal-companion-faded' : 'pointer-events-auto'}${ + napping ? ' petal-companion-sleep' : '' + }`} style={{ // Size scales with the viewport — see --petal-companion-size in index.css. width: 'var(--petal-companion-size)', diff --git a/web/src/components/Companion/useCardOverlap.ts b/web/src/components/Companion/useCardOverlap.ts new file mode 100644 index 0000000..6d5c363 --- /dev/null +++ b/web/src/components/Companion/useCardOverlap.ts @@ -0,0 +1,79 @@ +import { useEffect, useState, type RefObject } from 'react' + +// How often to re-measure outside of scroll/resize events. Cards re-pack when +// suggestions arrive, expand, or get accepted — none of which fire an event we +// can hear from here, so a slow poll picks those up. +const POLL_MS = 500 + +// What the mascot yields to. Suggestion cards stack down into its corner; the +// History and Garden drawers cover it outright, and their footer controls sat +// under the halo. Matching on the modal role rather than each panel's own class +// means a future drawer is covered the day it's written, without a list to keep +// in sync. Anything that doesn't actually reach the corner still won't trip the +// rect test below. +const CARD = '.petal-rail-card' +const MODAL = '[role="dialog"][aria-modal="true"]' + +export interface CardOverlap { + // A suggestion card reaches the mascot. It should get out of the way, but may + // still wake up when it has something to say. + cards: boolean + // A modal panel (History, Garden) covers the mascot's corner. Nothing the + // kitten wants to say is worth covering a dialog the writer opened on + // purpose, so this yields unconditionally. + modal: boolean +} + +// Reports what, if anything, the mascot should yield to at the given element. +// Used to fade the corner mascot out of the way when a card or panel reaches +// into its corner, so nothing is ever hidden (or made unclickable) by the +// kitten. +export function useCardOverlap(ref: RefObject): CardOverlap { + const [overlap, setOverlap] = useState({ cards: false, modal: false }) + + useEffect(() => { + let raf = 0 + + const check = () => { + const el = ref.current + if (!el) return + const r = el.getBoundingClientRect() + const hits = (selector: string) => { + for (const other of document.querySelectorAll(selector)) { + const b = other.getBoundingClientRect() + if (b.left < r.right && b.right > r.left && b.top < r.bottom && b.bottom > r.top) { + return true + } + } + return false + } + const next = { cards: hits(CARD), modal: hits(MODAL) } + // Same-value object identity would re-render on every poll tick. + setOverlap((prev) => + prev.cards === next.cards && prev.modal === next.modal ? prev : next, + ) + } + + const schedule = () => { + if (raf) return + raf = requestAnimationFrame(() => { + raf = 0 + check() + }) + } + + check() + // Capture phase so scrolls inside nested scrollers (History panel, rail) count. + window.addEventListener('scroll', schedule, true) + window.addEventListener('resize', schedule) + const timer = window.setInterval(check, POLL_MS) + return () => { + window.removeEventListener('scroll', schedule, true) + window.removeEventListener('resize', schedule) + window.clearInterval(timer) + cancelAnimationFrame(raf) + } + }, [ref]) + + return overlap +} diff --git a/web/src/index.css b/web/src/index.css index 6ef21e4..96be067 100644 --- a/web/src/index.css +++ b/web/src/index.css @@ -407,7 +407,17 @@ button, a, input { ~2× on a large desktop. Tune the middle (vw) term to taste. */ --petal-companion-size: clamp(10rem, 17vw, 20rem); animation: petal-bob 3.2s ease-in-out infinite; - transition: transform 200ms ease; + /* Shrink toward its corner when fading out of a card's way. `scale` is a + separate property from `transform` so it composes with the bob keyframes. */ + transform-origin: bottom right; + transition: transform 200ms ease, opacity 400ms ease, scale 400ms ease; +} +/* A suggestion card has drifted into the corner: the kitten politely turns + translucent, steps back 10%, and lets clicks pass through to the card. */ +.petal-companion-faded { + opacity: 0.15; + scale: 0.9; + pointer-events: none; } /* The Lottie art sits inside the round badge with a little breathing room. */ .petal-companion-art {