diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md index 840c079..29c1e10 100644 --- a/UX_REVIEW_2026-07-27.md +++ b/UX_REVIEW_2026-07-27.md @@ -302,6 +302,74 @@ 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. +### 4 — DONE (fifth session). The cards weren't distant; they were unreachable. + +Measured on the live build before touching anything, and the item understates +its own bug. Her open document: **four cards, 173 px each, all anchored inside +126 px of text** — the stack resolved to tops 4 / 189 / 374 / 559, so ~714 px +of cards beside four lines of prose. And because `.petal-rail` is +`position: absolute`, none of that counts as layout height: the page reported +`scrollHeight === clientHeight`, **no scroll container at all**. On the review's +810 px viewport the lower cards weren't merely severed from their sentence, +they were off-screen with no way to scroll to them. That is the real defect, +and it is why the item read as a scrolling problem. + +**Implemented:** + +- `SuggestionRail.tsx` — the stack reports how far it reaches (`onExtent`), + computed in the same pass that resolves the collision-avoided tops. +- `EditorCore.tsx` — the wrapper takes `minHeight: railExtent + 24`, so the + space the cards occupy becomes real, scrollable page. `minHeight` never + shrinks the column, so a rail that fits beside its text changes nothing. +- The prose moved into its own box, pinned with `position: sticky` while the + stack overhangs it, so scrolling down to reach the lower cards no longer + carries every sentence off the top. The offset is `min(0, port − content)`: + prose shorter than the viewport pins at the top; **taller prose pins by its + bottom edge**, so the last lines — the ones the overhanging cards flag — + stay visible rather than the first. +- The extent is cleared when the last card goes, or the window narrows past + the rail's threshold; otherwise the column keeps the height of a stack that + no longer exists. + +**A trap worth recording.** That prose box must be left at its natural height. +The first version kept the existing `h-full`, so it measured the wrapper — which +this change had just grown to the stack's height — and reported the cards' +height back as the text's own. `railExtent > contentH` was then never true and +the pin could never trip. It typechecked, looked right, and did nothing; only +measuring the running page caught it (`proseHeight: 1424` for a two-line +document). + +**Verified in a real browser at the review's own 1517×810**, driving the local +build with the rule pack from item 3b — which needs no model, so eight cards +appear offline in one paragraph. All three branches exercised: + +- *Overhang, short prose* — 8 cards, stack 1400 px, prose 95 px. Page gained + 675 px of scroll where it previously had none; scrolled to the end, the last + card sits fully in view (770–926) **and the prose is still on screen** (80–175). +- *Overhang, tall prose* — port 225 px, prose 347 px → `top: −146px`. Ordinary + scrolling is untouched (at `scrollTop` 200 the text moves normally with the + page); only at the overhang does it pin, bottom-anchored, last lines visible. +- *No overhang* — the port stays unscrollable and nothing moves. + +Hover-linking re-checked on the last card, the one this fix made reachable at +all: it glows the right span ("It make"), the span is on screen, the card lifts. + +**Known limit, not fixed.** The overlays anchored in wrapper coordinates (gloss +tip, selection bubble, word/misspell cards, confetti) rely on the invariant +noted at `recomputeRail` — "stable under scroll since text and wrapper scroll +together" — which the pin breaks. They are still placed correctly when opened, +because their coordinates come from live rects; they drift only if she scrolls +*while one is open* *and* the column is pinned, i.e. inside the overhang. Left +alone rather than papered over; if it ever bites, the fix is to close or +re-anchor them on scroll. + +**Deliberately not done:** no compaction of the cards. Making crowded cards +drop to a one-line form is the obvious way to shorten the stack, and it is +wrong here — the explanation *is* the teaching, and hiding it from an ESL +writer to save vertical space trades the product's purpose for tidiness. Ten +cards cannot sit beside four lines of text; the answer is to make the overhang +navigable, not to shrink what each card says. + ## 5. Mixed-language spans: offer translation, don't ignore **Status (follow-up session): premise partly wrong — re-scope before @@ -421,6 +489,12 @@ incremental-surfacing half is now cheap — the chunking it was waiting on exists — but it needs streaming, which the current `/check` shape doesn't do.)* +*(Fifth session: item 4 done — see the subsection under it. Still nothing +deployed: `main` remains at `ba06d90`, and 3b → 2 → 4 are now three stacked +topic branches. **Merging and deploying that stack is the obvious next move** +— three sessions of work she hasn't seen. Untouched: 6, 7, 8, item 3's +incremental half, and item 5's re-scoped Translate card type.)* + **Suggested next:** item 3b, the instant local rules layer — but it is **largely already built, in `main`**, and the item as written doesn't know that. Before writing any rules engine, read: diff --git a/web/src/App.tsx b/web/src/App.tsx index 95606cf..bc3fa11 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -518,7 +518,10 @@ export default function App() { <>
{/* Title, then the three chrome pills. Their labels are diff --git a/web/src/components/Editor/EditorCore.tsx b/web/src/components/Editor/EditorCore.tsx index 3e6fef5..0a5e317 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, useRef, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' import { Toolbar } from '../Toolbar/Toolbar' import { SuggestionCard } from './SuggestionCard' import { SuggestionRail, type RailItem } from './SuggestionRail' @@ -34,6 +34,10 @@ import { speak, speechSupported } from '../../audio/speech' import type { SpellChecker } from '../../hooks/useSpellChecker' import { usePack } from '../../i18n' +// Breathing room left below the last suggestion card when the rail's stack is what +// defines the column's height, so the bottom card doesn't sit flush on the edge. +const RAIL_TAIL = 24 + export interface EditorChange { content: string // Tiptap JSON, stringified content_text: string // flattened plain text for the LLM @@ -229,6 +233,9 @@ export function EditorCore({ // own name, so it says "português" rather than "pt-PT". const pack = usePack() const wrapperRef = useRef(null) + // The text column itself, measured separately from its wrapper: the wrapper is + // grown to cover the card stack, so only this reports the height of the prose. + const contentRef = useRef(null) const [hover, setHover] = useState(null) // The open spelling popover (click a red-underlined word), or null. const [misspell, setMisspell] = useState(null) @@ -266,6 +273,15 @@ export function EditorCore({ // `activeId` is the suggestion currently emphasized (hovered text or card). const [railItems, setRailItems] = useState([]) const [railEnabled, setRailEnabled] = useState(false) + // How far the resolved card stack reaches below the wrapper's top, reported by + // the rail. Cards are absolutely positioned and so contribute no layout height: + // without this the column below the last line of text isn't scrollable and any + // card that lands there is unreachable, not merely far from its sentence. + const [railExtent, setRailExtent] = useState(0) + // Sticky offset for the text column, or null when it should sit in normal flow. + // Set only while the stack overhangs the text: scrolling down to reach the lower + // cards would otherwise carry every sentence off the top of the screen. + const [stickTop, setStickTop] = useState(null) const [railExpandedId, setRailExpandedId] = useState(null) const [activeId, setActiveId] = useState(null) // A stable handle to the latest recompute so the editor's onUpdate (captured @@ -444,6 +460,42 @@ export function EditorCore({ } }, [recomputeRail]) + // The rail only reports its extent while it's mounted, so clear it when the last + // card goes (accepted the lot, or the window narrowed past the rail's threshold) + // — otherwise the column keeps the height of a stack that no longer exists. + useEffect(() => { + if (!railEnabled || railItems.length === 0) setRailExtent(0) + }, [railEnabled, railItems.length]) + + // Decide whether the text column has to be pinned. The rail's cards hang off an + // absolutely-positioned column, so when several suggestions share one short + // paragraph the stack runs far past the last line of text. Growing the wrapper to + // `railExtent` makes that space scrollable (item 4: the lower cards were simply + // unreachable); pinning the prose inside it means scrolling down to read those + // cards keeps the sentences on screen instead of scrolling them away. + // + // The offset is `min(0, port - content)`: prose shorter than the viewport sticks + // at the top, taller prose sticks by its *bottom* edge, so its last lines — the + // ones the overhanging cards flag — stay visible rather than the first. + useLayoutEffect(() => { + const wrapper = wrapperRef.current + const content = contentRef.current + if (!wrapper || !content || !railEnabled || railExtent <= 0) { + setStickTop(null) + return + } + const contentH = content.offsetHeight + // Only pin when the stack actually overhangs the prose; a rail that fits + // beside its text needs nothing, and pinning it would be a change for free. + if (railExtent <= contentH) { + setStickTop(null) + return + } + const port = wrapper.closest('.petal-scrollport') + const portH = port ? port.clientHeight : window.innerHeight + setStickTop(Math.min(0, portH - contentH - RAIL_TAIL)) + }, [railEnabled, railExtent, railItems]) + // Emphasize the flagged text for the active suggestion, mirroring the rail // card ↔ text link both ways. Driven through the decoration plugin (not an // imperative DOM class) so it survives the repaints that fire on every edit. @@ -1082,6 +1134,10 @@ export function EditorCore({
0 ? { minHeight: railExtent + RAIL_TAIL } : undefined} onMouseOver={handleMouseOver} onMouseOut={handleMouseOut} onMouseMove={handleMouseMove} @@ -1093,7 +1149,17 @@ export function EditorCore({ onTouchMove={cancelLongPress} onTouchEnd={cancelLongPress} > - + {/* The prose sits in its own box so it can be measured (and pinned) + independently of the wrapper, which the rail may have grown. The box is + deliberately left at its natural height: sized to the wrapper it would + report the stack's height back as the text's own, and the pin below + could never trip. */} +
+ +
{findOpen && editor && setFindOpen(false)} />} {confetti && } {gloss && ( @@ -1165,6 +1231,7 @@ export function EditorCore({ onHover={setActiveId} onActivate={activateRailCard} onToggleExpand={toggleRailExpand} + onExtent={setRailExtent} /> )}
diff --git a/web/src/components/Editor/SuggestionRail.tsx b/web/src/components/Editor/SuggestionRail.tsx index 3295edd..1b970a5 100644 --- a/web/src/components/Editor/SuggestionRail.tsx +++ b/web/src/components/Editor/SuggestionRail.tsx @@ -28,6 +28,12 @@ interface Props { // A card's body was clicked — scroll its highlight into view and toggle expand. onActivate: (id: string) => void onToggleExpand: (id: string) => void + // How far down the resolved stack reaches (px below the wrapper's top). Cards + // are absolutely positioned, so they add nothing to layout height — a cluster of + // errors in one short paragraph can pile cards hundreds of px past the end of the + // text, with no scrollable space to reach them. The editor uses this to grow the + // column so every card can at least be scrolled to. + onExtent: (bottom: number) => void } // SuggestionRail is the right-margin "comment column": every outstanding @@ -44,6 +50,7 @@ export function SuggestionRail({ onHover, onActivate, onToggleExpand, + onExtent, }: Props) { // Measured resolved tops keyed by suggestion id (after collision avoidance). const [tops, setTops] = useState>({}) @@ -65,13 +72,16 @@ export function SuggestionRail({ const layoutKey = ordered.map((i) => `${i.suggestion.id}:${Math.round(i.anchorTop)}`).join('|') useLayoutEffect(() => { let cursor = -Infinity + let bottom = 0 const next: Record = {} for (const { suggestion, anchorTop } of ordered) { const h = cardRefs.current.get(suggestion.id)?.offsetHeight ?? 96 const top = Math.max(anchorTop, cursor) next[suggestion.id] = top cursor = top + h + CARD_GAP + bottom = top + h } + onExtent(bottom) setTops((prev) => { const ids = Object.keys(next) if (ids.length === Object.keys(prev).length && ids.every((id) => prev[id] === next[id])) return prev