Kitten yields to panels too; redo bug not reproducible
The overlap hook only watched .petal-rail-card, so the History and Garden
drawers still sat under the mascot — with a real control ("写作证明 ·
Writing passport") buried under the halo on the live build.
Match [role="dialog"][aria-modal="true"] as well. Both drawers already
render it, so this covers them and any future drawer without a selector
list to keep in sync.
Two things the follow-up 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.
Hold it back while a panel is open; useCompanion keeps it in state, so
it reappears when she closes the panel.
Also guard the poll's setState on value equality, so the 500 ms tick
stops re-rendering the companion for an unchanged answer.
UX_REVIEW item 1 (redo does not re-apply an accepted suggestion) is
recorded as NOT REPRODUCIBLE. Read the prosemirror-history state directly
and hooked view.dispatch: redo works pressed immediately, after an 18 s
pause that lets a full re-check land, and with the editor never focused.
The doc's hypothesis is false — every re-check transaction is
decoration-only, which prosemirror-history ignores, and canRedo stayed
true throughout. Two real findings from that dig are written into the doc
instead: keyboard undo dies when focus isn't in the editor, and an undone
suggestion stays accepted server-side so its card doesn't reliably return.
Item 5's premise is also partly wrong and now re-scoped: the Chinese
sentence does produce a card with an English rendering, just labeled
Clarity rather than a first-class Translate type.
Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua
This commit is contained in:
+71
-11
@@ -39,21 +39,71 @@ companion picker is open, or when cards no longer overlap.
|
|||||||
- `web/src/components/Companion/PetalCompanion.tsx` — `faded` wiring.
|
- `web/src/components/Companion/PetalCompanion.tsx` — `faded` wiring.
|
||||||
- `web/src/index.css` — `.petal-companion-faded`, transition.
|
- `web/src/index.css` — `.petal-companion-faded`, transition.
|
||||||
|
|
||||||
**Follow-up for Opus:** the overlap check only watches `.petal-rail-card`.
|
**Follow-up — DONE.** The overlap check now watches
|
||||||
The History panel footer and Garden counter can still sit under the kitten.
|
`.petal-rail-card, [role="dialog"][aria-modal="true"]`. Matching the modal
|
||||||
Either extend the hook's selector list (`.petal-rail-card, [data-panel]`…)
|
role rather than each panel's class covers History and Garden today (both
|
||||||
or give those panels a bottom padding of
|
already render `role="dialog" aria-modal="true"` drawers) and any future
|
||||||
`calc(var(--petal-companion-size) * 0.5)`. Acceptance: with History open
|
drawer for free, with no selector list to keep in sync.
|
||||||
and with the Garden footer visible, no interactive control is under the
|
|
||||||
mascot, or the mascot is faded + click-through.
|
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
|
## 1. Bug: redo does not re-apply an accepted suggestion — NOT REPRODUCIBLE
|
||||||
|
|
||||||
**Repro:** accept a suggestion (text updates), press Ctrl+Z (text reverts —
|
**Status (2026-07-27, follow-up session):** investigated against the live
|
||||||
correct), press Ctrl+Shift+Z → nothing happens. Observed on the Idiom card
|
VPS build with the ProseMirror history plugin state read directly
|
||||||
"by foots → on foot".
|
(`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`
|
**Where to look:** `EditorCore.handleAccept`
|
||||||
(`web/src/components/Editor/EditorCore.tsx` ~line 556) applies via
|
(`web/src/components/Editor/EditorCore.tsx` ~line 556) applies via
|
||||||
@@ -133,6 +183,16 @@ scrolling the card list; hover-linking still highlights the right span.
|
|||||||
|
|
||||||
## 5. Mixed-language spans: offer translation, don't ignore
|
## 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
|
Typed mid-document: 我想说这句话但是不知道用英语怎么说。 ("I want to say
|
||||||
this but don't know how in English") — Petal produced **no card at all**.
|
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,
|
The pair model (`SUGGESTIONS.md` §1: user may type in either language,
|
||||||
|
|||||||
@@ -92,9 +92,10 @@ export function PetalCompanion({
|
|||||||
// When suggestion cards stack down into the corner, the kitten fades to
|
// When suggestion cards stack down into the corner, the kitten fades to
|
||||||
// translucent and shrinks a step so the card stays readable and clickable.
|
// 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
|
// It wakes back up whenever it has something to say (bubble) or is being
|
||||||
// interacted with (picker open).
|
// 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 crowded = useCardOverlap(badgeRef)
|
||||||
const faded = crowded && !pickerOpen && !bubble
|
const faded = crowded.modal || (crowded.cards && !pickerOpen && !bubble)
|
||||||
|
|
||||||
// Awake companions (no sleeping clip) don't visibly nap — when the engine
|
// 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
|
// dozes them, keep their normal idle pose instead of a sleepy face. Only a
|
||||||
@@ -185,7 +186,10 @@ export function PetalCompanion({
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{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 && (
|
||||||
<div
|
<div
|
||||||
role="status"
|
role="status"
|
||||||
onClick={dismiss}
|
onClick={dismiss}
|
||||||
|
|||||||
@@ -5,11 +5,31 @@ import { useEffect, useState, type RefObject } from 'react'
|
|||||||
// can hear from here, so a slow poll picks those up.
|
// can hear from here, so a slow poll picks those up.
|
||||||
const POLL_MS = 500
|
const POLL_MS = 500
|
||||||
|
|
||||||
// True while any suggestion card (.petal-rail-card) overlaps the given element.
|
// What the mascot yields to. Suggestion cards stack down into its corner; the
|
||||||
// Used to fade the corner mascot out of the way when the rail grows down into
|
// History and Garden drawers cover it outright, and their footer controls sat
|
||||||
// its corner, so a card is never hidden (or made unclickable) by the kitten.
|
// under the halo. Matching on the modal role rather than each panel's own class
|
||||||
export function useCardOverlap(ref: RefObject<HTMLElement | null>): boolean {
|
// means a future drawer is covered the day it's written, without a list to keep
|
||||||
const [overlapped, setOverlapped] = useState(false)
|
// 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<HTMLElement | null>): CardOverlap {
|
||||||
|
const [overlap, setOverlap] = useState<CardOverlap>({ cards: false, modal: false })
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
let raf = 0
|
let raf = 0
|
||||||
@@ -18,15 +38,20 @@ export function useCardOverlap(ref: RefObject<HTMLElement | null>): boolean {
|
|||||||
const el = ref.current
|
const el = ref.current
|
||||||
if (!el) return
|
if (!el) return
|
||||||
const r = el.getBoundingClientRect()
|
const r = el.getBoundingClientRect()
|
||||||
let hit = false
|
const hits = (selector: string) => {
|
||||||
for (const card of document.querySelectorAll('.petal-rail-card')) {
|
for (const other of document.querySelectorAll(selector)) {
|
||||||
const b = card.getBoundingClientRect()
|
const b = other.getBoundingClientRect()
|
||||||
if (b.left < r.right && b.right > r.left && b.top < r.bottom && b.bottom > r.top) {
|
if (b.left < r.right && b.right > r.left && b.top < r.bottom && b.bottom > r.top) {
|
||||||
hit = true
|
return true
|
||||||
break
|
}
|
||||||
}
|
}
|
||||||
|
return false
|
||||||
}
|
}
|
||||||
setOverlapped(hit)
|
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 = () => {
|
const schedule = () => {
|
||||||
@@ -50,5 +75,5 @@ export function useCardOverlap(ref: RefObject<HTMLElement | null>): boolean {
|
|||||||
}
|
}
|
||||||
}, [ref])
|
}, [ref])
|
||||||
|
|
||||||
return overlapped
|
return overlap
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user