Skip to content

Commit e1f0df5

Browse files
fix(preview): block-editing glitches round 3 (G20–G22)
G20: nested list-item editor height uses the range's first client rect, not the bounding union, so the editor matches the rendered line. G21: committing a nested item keeps focus on the edited surface (mode-aware refocusTargetForAnchorR0) instead of jumping to the next block. G22: commit-status bulb + commit-error routing in the preview. Includes the boundary-splice design, the G20–G22 clean-slate spec, and the hub-client changelog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 186c989 commit e1f0df5

11 files changed

Lines changed: 2690 additions & 14 deletions

claude-notes/plans/2026-06-18-block-editing-glitches-3.md

Lines changed: 763 additions & 0 deletions
Large diffs are not rendered by default.

claude-notes/plans/2026-06-18-boundary-splice-edit-design.md

Lines changed: 362 additions & 0 deletions
Large diffs are not rendered by default.

claude-notes/plans/2026-06-19-boundary-splice-implementation.md

Lines changed: 1007 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# Item plane — research-level plan (DEFERRED)
2+
3+
**Date:** 2026-06-19
4+
**Branch:** TBD (follow-on to `feature/block-editing-improvements`)
5+
**Status:** RESEARCH / DEFERRED — not scheduled. Sibling of the **block plane**
6+
(`2026-06-18-boundary-splice-edit-design.md` + `2026-06-19-boundary-splice-implementation.md`).
7+
8+
> This is a research stub, not an implementation plan. It captures the problem,
9+
> the open design questions, and rough estimates so the work can be picked up
10+
> cleanly. Do **not** start implementing from this file — it must first go
11+
> through brainstorming → spec → implementation plan like the block plane did.
12+
13+
## What the item plane is
14+
15+
The block plane splices **`Block`s within a `Blocks` slice** (`Vec<Block>`). The
16+
item plane splices the *other* element type that lists/tables/def-lists are built
17+
from:
18+
19+
```rust
20+
BulletList { content: Vec<Vec<Block>> } // elements: ITEMS (Vec<Block>)
21+
OrderedList { content: Vec<Vec<Block>>, .. }
22+
DefinitionList { content: Vec<(Vec<Inline> /*term*/, Vec<Vec<Block>> /*bodies*/)> }
23+
Table { ... rows of cells ... } // elements: ROWS
24+
```
25+
26+
Operations: **insert / move / delete an item** (list item, table row, def-list
27+
entry) — splices on `Vec<Vec<Block>>` (or the table row vector), not on a
28+
`Blocks` slice. "Add a kanban card", "drag a card to position 4", "add a row",
29+
"add a definition" all live here.
30+
31+
## Why it was split out of the block plane (decided 2026-06-19)
32+
33+
- **Different element type.** A list item is `Vec<Block>`, not a `Block`. The
34+
block plane's `splice_range` operates on `Vec<Block>`; items are one level up.
35+
- **Different content shape.** A new item's payload is "the blocks that make up
36+
the item" — and multi-item inserts need item delimiting. The block plane's
37+
`Content` (`md`/`ast` → blocks) does not express "these blocks form item k,
38+
those form item k+1".
39+
- **It is the only thing that wanted a positional slot among sibling item-slices**
40+
— i.e. the positional `item`/`(def,body)` coordinates we deliberately kept out
41+
of the block plane's `ContainerRef`. Putting them here keeps the block plane
42+
fully SI-addressed and index-free.
43+
- **No current consumer needs a primitive.** Today: typed list editing goes
44+
through the text channel (qmd reparse builds items for free); `kanban.tsx`
45+
reorders via `replaceNode`-the-whole-list. So the item plane is real future
46+
work, not blocking.
47+
48+
## How it composes with the block plane (additivity check)
49+
50+
The split is additive. The item plane adds its **own boundary family** over
51+
item-arrays; it does not change the block plane's `Boundary`/`ContainerRef`. A
52+
gesture that fills a freshly-inserted item is "insert item *with content*" (one
53+
item-plane op), not "insert empty item then block-splice into it" — so the block
54+
plane never needs `listItem`/`defBody` back.
55+
56+
Sketch (to be designed, not final):
57+
58+
```
59+
ItemBoundary = beforeItem(listSi, k) | afterItem(listSi, k)
60+
| startOfList(listSi) | endOfList(listSi) // gap 0 / item-count
61+
ItemContent = md(...) parsed as items | items(blockGroups...) // shape TBD
62+
insertItemAt(listSi, k, itemContent) // { from: beforeItem(listSi,k), to: ..., itemContent }
63+
moveItem(listSi, from, to) // delete + insert, or a dedicated op
64+
```
65+
66+
`listSi` SI-matches the specific (possibly nested) list at any depth — depth is
67+
absorbed by SI exactly as in the block plane; only *one* shallow index per list
68+
level, never a root path.
69+
70+
## Open design questions (the research)
71+
72+
1. **Item content shape (the hard one).** How does `md('- a\n- b')` map to
73+
*items*? How does the `ast` form express "two items"? Single- vs multi-item
74+
payloads and delimiting. Does it reuse `Content` or get its own type?
75+
2. **Tight vs loose lists.** Inserting into a tight list must stay tight — the
76+
item-level analog of the block plane's `preserve_leaf_variant` (Plain↔Para).
77+
How is tight/loose detected and preserved on insert?
78+
3. **Ordered-list semantics.** Renumbering, `start`/`style`/`delim` interaction
79+
with the incremental writer when items are inserted/removed/reordered.
80+
4. **Scope: which containers.** Bullet/ordered lists first? Then def-list entries
81+
(need a *term*, and bodies are `Vec<Vec<Block>>` — two coordinates) and table
82+
rows (fixed column count → row shape validation). Each is its own sub-design.
83+
5. **Reconciler/writer behavior.** Confirm `compute_reconciliation` handles item
84+
count changes cleanly (the `list_*_produces_correct_length` tests in
85+
`quarto-ast-reconcile` suggest yes) and that the incremental writer's
86+
whole-list re-serialization is acceptable (it is the same nested-edit behavior
87+
the block plane already accepts).
88+
6. **Move semantics.** Is "move item" a first-class op (so reconciliation can
89+
keep source fidelity of the moved item) or just delete+insert? Relevant to
90+
kanban drag.
91+
7. **Frontend coordinates.** A list-level component (kanban) has `itemIndex`
92+
positionally; a component nested *inside* an item does not (item indices are
93+
render-time-local — confirmed 2026-06-19). Decide whether to expose item
94+
indices via context, or keep item-plane gestures list-level only.
95+
96+
## Rough estimates (ideal hours, assuming the block plane is in place)
97+
98+
| Scope | Planning | Engineering |
99+
|---|---|---|
100+
| Bullet/ordered lists | 3–5h | 8–14h |
101+
| + def-list entries + table rows | +2–3h | +8–12h |
102+
103+
Biggest swing risks: the **item content shape** (Q1), **tight/loose + ordered
104+
renumbering** (Q2/Q3), and whether migrating `kanban.tsx` from `replaceNode` to
105+
item-moves surfaces reconciler quirks (Q5/Q6).
106+
107+
## Entry criteria
108+
109+
Pick this up when a real gesture needs a structural item primitive (e.g. kanban
110+
drag wanting surgical item moves instead of whole-list `replaceNode`, or a
111+
nesting-cursor "new list item" that we decide should be structural rather than
112+
text-reparse). Until then it stays deferred. Start with brainstorming (the
113+
content shape, Q1, deserves a real design pass), not with code.

hub-client/changelog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ be in reverse chronological order (latest first).
1515
1616
-->
1717

18+
### 2026-06-19
19+
20+
- [`c38ca41b`](https://github.com/quarto-dev/q2/commits/c38ca41b): Block-editing fixes (G20–G22): the in-place editor for a nested list item no longer opens taller than the line it replaces; committing a nested-item edit now keeps the edited node selected instead of jumping focus to the next block; and a new commit-status indicator shows whether each edit was a real change or a spurious no-op, surfacing edit errors in the preview.
21+
1822
### 2026-06-18
1923

2024
- [`8f6d5a0c`](https://github.com/quarto-dev/q2/commits/8f6d5a0c): Block-editing fixes (G14–G19): the nesting cursor is now ON by default; single-line editors no longer inflate to two lines on expand; down-arrow steps correctly out of a blockquote-wrapped loose list; clicking between items reliably activates on the first click with nesting kept live; a stuck "blur the cell you left" effect now always clears; and a spurious no-op write when clicking off an untouched nested block is eliminated.

hub-client/src/components/render/ReactPreview.tsx

Lines changed: 169 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useState, useCallback, useRef, useEffect, useMemo } from 'react';
2+
import type { CSSProperties } from 'react';
23
import type * as Monaco from 'monaco-editor';
34
import type { FileEntry } from '@quarto/preview-renderer/types/project';
45
import type { Diagnostic, PreviewNodeEditPayload } from '@quarto/preview-renderer/types/diagnostic';
@@ -281,6 +282,113 @@ async function doRender(
281282
}
282283
}
283284

285+
// Exported for unit test. SPURIOUS = the edit round-tripped to identical QMD.
286+
export function classifyCommitOutcome(
287+
newQmd: string,
288+
renderedContent: string,
289+
): 'change' | 'spurious' {
290+
return newQmd === renderedContent ? 'spurious' : 'change';
291+
}
292+
293+
/**
294+
* G22: commit-status indicator — a translucent "glass" status dot in the
295+
* preview's bottom-right corner that glows out of the page and fades to nothing.
296+
*
297+
* pending → amber (commit going out)
298+
* change → green (real diff written)
299+
* spurious → blue (no-op round-trip — the false-dirty bug class)
300+
* error → (not lit here; the error pill takes over the corner)
301+
*
302+
* Modern / non-skeuomorphic: no bezel, no fixture, no persistence. The dot is
303+
* defined by LIGHT not by a housing — a colored radial bloom, a thin colored
304+
* glass rim, and a `backdrop-filter` refraction — so it reads on a white page
305+
* now and on a dark background later. Idle → opacity 0 (gone). Lit states bloom
306+
* in (scale + fade) and dim back out; only the attention state (blue) pulses.
307+
*/
308+
const COMMIT_BULB_CSS = `
309+
@keyframes q2-bulb-pulse {
310+
0%, 100% { opacity: 0.84; }
311+
50% { opacity: 1; }
312+
}
313+
.q2-commit-bulb {
314+
position: absolute;
315+
/* Sits in the bottom-right corner where the error pill appears, so on error
316+
the pill visually replaces the bulb. */
317+
bottom: 20px;
318+
right: 20px;
319+
width: 13px;
320+
height: 13px;
321+
border-radius: 50%;
322+
background: radial-gradient(circle at 50% 45%, var(--c-bright) 0%, var(--c-soft) 58%, transparent 100%);
323+
border: 1px solid var(--c-rim);
324+
backdrop-filter: blur(3px) saturate(1.4);
325+
-webkit-backdrop-filter: blur(3px) saturate(1.4);
326+
box-shadow: 0 0 var(--c-glow-blur) var(--c-glow-spread) var(--c-glow);
327+
opacity: var(--bulb-opacity);
328+
transform: scale(var(--bulb-scale));
329+
transition:
330+
opacity 300ms ease,
331+
box-shadow 220ms ease,
332+
background 220ms ease,
333+
border-color 220ms ease,
334+
transform 300ms cubic-bezier(0.2, 0.7, 0.3, 1.3);
335+
pointer-events: none;
336+
z-index: 50;
337+
will-change: opacity, transform;
338+
}
339+
/* Gentle, slow breath — only the attention state (blue) pulses. */
340+
.q2-commit-bulb[data-pulse="1"] { animation: q2-bulb-pulse 1.5s ease-in-out infinite; }
341+
`;
342+
343+
export function CommitStatusBulb({
344+
status,
345+
}: {
346+
status: 'idle' | 'pending' | 'change' | 'spurious' | 'error';
347+
}) {
348+
// RGB triples + per-state "loudness" — green (a normal change) is quiet; blue
349+
// (spurious) glows harder, pulses, and lingers so it pulls the eye. Derived
350+
// from one hue so it carries to light and dark backgrounds.
351+
//
352+
// 'error' is intentionally NOT lit here: on error the bulb hands off to the
353+
// error pill (PreviewErrorOverlay), which appears in the same corner — see the
354+
// render below where the bulb is suppressed while a commit error is showing.
355+
const PALETTE: Record<
356+
'pending' | 'change' | 'spurious',
357+
{ rgb: string; glowA: number; blur: string; spread: string; pulse: boolean; title: string }
358+
> = {
359+
pending: { rgb: '255,160,30', glowA: 0.42, blur: '9px', spread: '0px', pulse: false, title: 'Committing…' },
360+
change: { rgb: '40,200,100', glowA: 0.36, blur: '8px', spread: '0px', pulse: false, title: 'Change committed' },
361+
spurious: { rgb: '60,140,255', glowA: 0.6, blur: '13px', spread: '1px', pulse: true, title: 'No change (spurious edit)' },
362+
};
363+
const on = status === 'pending' || status === 'change' || status === 'spurious';
364+
const p = on ? PALETTE[status] : null;
365+
const rgb = p?.rgb ?? '128,128,128';
366+
return (
367+
<>
368+
<style>{COMMIT_BULB_CSS}</style>
369+
<div
370+
className="q2-commit-bulb"
371+
data-pulse={p?.pulse ? '1' : undefined}
372+
title={p?.title}
373+
style={
374+
{
375+
'--c-bright': `rgba(${rgb},0.92)`,
376+
'--c-soft': `rgba(${rgb},0.30)`,
377+
'--c-rim': `rgba(${rgb},0.55)`,
378+
'--c-glow': `rgba(${rgb},${p?.glowA ?? 0})`,
379+
'--c-glow-blur': p?.blur ?? '0px',
380+
'--c-glow-spread': p?.spread ?? '0px',
381+
// No persistence: idle fades to nothing and shrinks slightly so lit
382+
// states bloom back in.
383+
'--bulb-opacity': on ? 1 : 0,
384+
'--bulb-scale': on ? 1 : 0.55,
385+
} as CSSProperties
386+
}
387+
/>
388+
</>
389+
);
390+
}
391+
284392
export default function ReactPreview({
285393
content,
286394
currentFile,
@@ -391,6 +499,51 @@ export default function ReactPreview({
391499
const renderTimeoutRef = useRef<number | null>(null);
392500
const lastContentRef = useRef<string>('');
393501

502+
// G22: commit-status indicator. Every commit channel (text / subtree /
503+
// nesting) funnels through handleSetAst, so we classify the outcome there and
504+
// drive ONE bulb overlay from a single status state — no per-site plumbing.
505+
const [commitStatus, setCommitStatus] = useState<
506+
'idle' | 'pending' | 'change' | 'spurious' | 'error'
507+
>('idle');
508+
const commitStatusTimerRef = useRef<number | null>(null);
509+
const commitPendingSinceRef = useRef<number>(0);
510+
// G22: the message for a rejected commit, surfaced in the existing
511+
// PreviewErrorOverlay. Persists until the next SUCCESSFUL commit.
512+
const [commitError, setCommitError] = useState<string | null>(null);
513+
514+
// Light the bulb amber when a commit goes out.
515+
const beginCommitStatus = useCallback(() => {
516+
if (commitStatusTimerRef.current !== null) clearTimeout(commitStatusTimerRef.current);
517+
commitPendingSinceRef.current = Date.now();
518+
setCommitStatus('pending');
519+
}, []);
520+
521+
// applyNodeEdit is synchronous, so hold 'pending' a minimum so it is visible,
522+
// then flip to the result colour, then auto-clear to idle.
523+
const settleCommitStatus = useCallback(
524+
(result: 'change' | 'spurious' | 'error') => {
525+
const MIN_PENDING_MS = 200;
526+
// Loudness via dwell time: a normal change blinks briefly; a spurious /
527+
// rejected commit lingers longer so it draws the eye.
528+
const HOLD_MS = { change: 450, spurious: 1100, error: 1700 }[result];
529+
const elapsed = Date.now() - commitPendingSinceRef.current;
530+
const delay = Math.max(0, MIN_PENDING_MS - elapsed);
531+
if (commitStatusTimerRef.current !== null) clearTimeout(commitStatusTimerRef.current);
532+
commitStatusTimerRef.current = window.setTimeout(() => {
533+
setCommitStatus(result);
534+
commitStatusTimerRef.current = window.setTimeout(() => {
535+
setCommitStatus('idle');
536+
commitStatusTimerRef.current = null;
537+
}, HOLD_MS);
538+
}, delay);
539+
},
540+
[],
541+
);
542+
543+
useEffect(() => () => {
544+
if (commitStatusTimerRef.current !== null) clearTimeout(commitStatusTimerRef.current);
545+
}, []);
546+
394547
// Handler for cross-document navigation
395548
const handleNavigateToDocument = useCallback(
396549
(targetPath: string, anchor: string | null) => {
@@ -522,13 +675,17 @@ export default function ReactPreview({
522675
);
523676
return;
524677
}
678+
// G22: a commit is going out — light the bulb amber.
679+
beginCommitStatus();
525680
try {
526681
let modifiedSubtreeJson: string;
527682
if (edit.channel === 'text') {
528683
// Text channel: parse raw QMD → Pandoc JSON → apply_node_edit.
529684
const parseResult = parseQmdContentSync(edit.newText);
530685
if (!parseResult.success || !parseResult.ast) {
531686
console.error('parse_qmd_content failed:', parseResult.error);
687+
settleCommitStatus('error'); // G22: parse rejected
688+
setCommitError(`Edit could not be parsed: ${parseResult.error ?? 'unknown error'}`);
532689
return;
533690
}
534691
modifiedSubtreeJson = parseResult.ast;
@@ -544,9 +701,15 @@ export default function ReactPreview({
544701
edit.destinationSourceInfoJson,
545702
modifiedSubtreeJson,
546703
);
704+
// G22: a commit that round-trips to identical QMD is a SPURIOUS edit
705+
// (the false-dirty bug class) — surface it instead of silently writing.
706+
settleCommitStatus(classifyCommitOutcome(newQmd, rendered.renderedContent));
707+
setCommitError(null); // G22: a successful commit clears the prior error
547708
onContentRewrite(newQmd);
548709
} catch (err) {
549710
console.error('apply_node_edit failed:', err);
711+
settleCommitStatus('error'); // G22: apply rejected
712+
setCommitError(`Edit could not be applied: ${err instanceof Error ? err.message : String(err)}`);
550713
}
551714
return;
552715
}
@@ -557,11 +720,14 @@ export default function ReactPreview({
557720
console.error('Failed to write AST back to QMD:', err);
558721
}
559722
},
560-
[content, onContentRewrite, format, rendered.untransformedAstJson, rendered.renderedContent],
723+
[content, onContentRewrite, format, rendered.untransformedAstJson, rendered.renderedContent, beginCommitStatus, settleCommitStatus],
561724
);
562725

563726
return (
564727
<div style={{ height: '100%', display: 'flex', flexDirection: 'column', position: 'relative' }}>
728+
{/* Bulb lives in the same bottom-right corner as the error pill; while a
729+
commit error is showing, the pill replaces the bulb (bulb → idle). */}
730+
<CommitStatusBulb status={commitError ? 'idle' : commitStatus} />
565731
<div style={{ flex: 1, position: 'relative', overflow: 'hidden' }}>
566732
{rendered.astJson && (previewState === 'GOOD' || previewState === 'ERROR_FROM_GOOD') ? (
567733
<ReactRenderer
@@ -596,8 +762,8 @@ export default function ReactPreview({
596762
</div>
597763
{/* Error overlay shown when error occurs after successful render */}
598764
<PreviewErrorOverlay
599-
error={currentError}
600-
visible={previewState === 'ERROR_FROM_GOOD'}
765+
error={currentError ?? (commitError ? { message: commitError } : null)}
766+
visible={previewState === 'ERROR_FROM_GOOD' || commitError != null}
601767
collapsed={errorOverlayCollapsed}
602768
onToggleCollapsed={setErrorOverlayCollapsed}
603769
/>

0 commit comments

Comments
 (0)