feat(maic-editor): selection-anchored text editing for the slide surface#590
Merged
cosarah merged 40 commits intoMay 24, 2026
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…d bar) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add useResolvedSlideContent / useEditingTextElementId / useSyncEditingElementId. Realign the PR2 buildFloatingActions tests with the new behavior (text formatting moved off the FloatingToolbar) and co-locate the editing-state test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nvas Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…g edited Gated on the canvas store's editingElementId (default ""), so the dashed select frame is unchanged for multi-select and for any consumer that never sets the flag. Editor-path only; playback never renders Operate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…e frame Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…apper
Code review caught that #editable-element-{id} is a zero-size absolute
wrapper — measuring it would pin the bar to the canvas origin. Measure the
.editable-element-text child, which carries the real geometry. Also correct
the dismiss-behavior comment: the bar is purely selection-driven.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the native <select> font picker with the design-system Select, rebuild the size control as one cohesive stepper pill, swap the color "A" for a swatch chip, and unify every control to a single height and hover/ active language (violet accent, matching the editor's Pro-mode accent). Behavior and the text commands are unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
configs/font.ts listed 29 fonts but the app only ever loads Inter (via next/font); the other 28 had no @font-face or bundled file, so picking them silently fell back with no visible effect — and nothing but the format bar even imports the registry. Trim it to what genuinely renders; the file's comment records how to restore the rest (wire up font loading first). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The font registry listed 29 fonts the app never loaded. Wire up a curated set that genuinely renders — 思源黑/宋, 霞鹜文楷, 站酷快乐体, and 9 Latin families — via @fontsource packages (npm-managed, no font binaries in the repo; CJK faces are unicode-range-subsetted so they download lazily per glyph range). app/editor-fonts.ts registers the @font-face CSS from the root layout; configs/font.ts is now the real, honest 14-entry list. The ~14 commercial decorative Chinese fonts are intentionally left out — they need self-hosting + subsetting + a licensing review, separate work. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Picking a font whose family name has spaces or a trailing digit (e.g.
"Source Sans 3") threw `Failed to execute 'check' on 'FontFaceSet'` —
`document.fonts.check(\`16px ${name}\`)` needs the family quoted — and the
fontname mark's toDOM emitted an invalid unquoted `font-family`, so the
font silently never applied. Quote the family in both spots; the mark's
parseDOM already strips quotes, so the attr still round-trips clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The clean editing frame is a purely visual full-size overlay, but it was pointer-events: auto — so it masked the text element's own move cursor, text cursor, click-to-place-caret and drag-to-move; only a thin uncovered sliver at the edges still triggered them. The dashed BorderLines it replaced are thin edge lines, so they never had this problem. Mark the frame pointer-events-none; the resize/rotate handles are separate and keep their own pointer events. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
With @fontsource fonts and font-display: swap, a picked font swaps in smoothly on its own — the "Font is loading, please wait..." toast was noise (and fired on most CJK picks while a unicode-range chunk loaded). Remove it along with the now-unused document.fonts.check and toast import. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A text element's contextual actions now sit together on the anchored bar — format controls + delete, hugging the element — instead of delete sitting alone in the top-center FloatingToolbar. buildFloatingActions returns nothing for text (its FloatingToolbar then renders null); non-text elements still get their delete there. Delete logic is shared via a new deleteSlideElement helper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A selected image element now gets a selection-anchored bar hugging it — just a delete button (image replace/crop/flip stay in a later sub-PR) — the same way text elements do. The anchoring shell is extracted out of AnchoredTextBar into a reusable AnchoredBar, and the delete button into a shared DeleteButton; AnchoredTextBar and the new AnchoredImageBar are thin wrappers. useTrackedRect now measures .editable-element-text or .editable-element-image. buildFloatingActions returns nothing for image elements too (other element types still get their delete there). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
p-2 left a chunky white margin around the content — most visible on the image bar, a lone delete button in an oversized box. p-1 (4px, the value the FloatingToolbar used) makes both bars sit snug to their controls. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The selection-anchored delete bar now covers all non-text element types
(shape, line, table, chart, …), not just image — so every element's
editing chrome is anchored uniformly. AnchoredImageBar becomes the
type-agnostic AnchoredDeleteBar; useTrackedRect matches any
.editable-element-{type} content root; buildFloatingActions is dropped —
the surface no longer contributes top-center FloatingToolbar actions,
everything is on an anchored bar.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
cosarah
reviewed
May 22, 2026
Collaborator
cosarah
left a comment
There was a problem hiding this comment.
Issues
Important
components/edit/surfaces/slide/text-format-bar.tsx:125—<SelectValue />has no placeholder. Legacyattrs.fontnameoutside curatedFONTS(e.g.Microsoft YaHei,PingFang SC,Arial) renders a blank trigger. Fix:<SelectValue placeholder={attrs.fontname || t('edit.text.fontDefault')} />, or inject a disabled "current" item when unmatched.
Minor
lib/prosemirror/schema/marks.ts:140—font-family: "${fontname}"does not escape".parseDOMstrips quotes so paste round-trips are safe, but a hand-crafted slide JSONfontname: 'X"; background:url(...);"injects CSS attoDOM. Defence-in-depth — escape"or reject/["\\]/.app/editor-fonts.ts— 8@fontsourcepackages as side-effect imports viaapp/layout.tsx. Font binaries stay lazy, but@font-faceCSS ships on every page. Considerawait import('./editor-fonts')from insideSlideCanvasif editor is rarely opened.components/edit/surfaces/slide/use-tracked-rect.ts:44-59— Unconditional 60HzrequestAnimationFrame.sameRectshort-circuitssetState, cost is onegetBoundingClientRectper frame.ResizeObserver+ scroll listener would drop idle CPU.components/edit/surfaces/slide/AnchoredBar.tsx:26—<Popover open={open}>controlled withoutonOpenChange. Documented as intentional, but Radix logs dev warning andEsc-dismiss for SR users is lost.components/edit/surfaces/slide/use-slide-surface.ts:228-236— TwouseLayoutEffects onsetEditingElementId; cleanup-only effect duplicates the first effect's dep-change cleanup. Foldable to one effect withreturn () => setEditingElementId('').configs/font.ts:14—label: '默认字体'is dead —text-format-bar.tsx:130overrides witht('edit.text.fontDefault'). Drop the label or the override.
When a text element's `fontname` was a value not in the curated FONTS registry (e.g. `Microsoft YaHei`, `PingFang SC`, theme defaults), the Select couldn't match it and `<SelectValue/>` rendered a blank trigger — both reviewers (cosarah Important, xuyuanwei678 #1) caught this. Add a placeholder fallback so the raw family name surfaces in the trigger. Also clean up the dead `'默认字体'` label that `text-format-bar.tsx` overrode unconditionally: introduce an optional `labelKey` field on `FontEntry`, use it for the default entry, and let the picker prefer the i18n key when present — no more by-value special case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- `marks.ts` fontname `toDOM` rejects `"` or `\` instead of interpolating them: a hand-crafted mark with `fontname: 'X"; background:url(...);'` could otherwise close the quoted string and inject arbitrary CSS. - `AnchoredBar` gains `onOpenChange` (clears the canvas selection on Radix-initiated dismiss): silences the controlled-without-handler dev warning, and brings back Esc / SR dismissal that our focus-outside hardening had cut off. - `useSyncEditingElementId` folds two `useLayoutEffect`s into one with a cleanup; the previous unmount-only effect was structural noise. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Radix Select / Popover wrap with `react-remove-scroll`, which adds a compensation `padding-right` to <body> when they open. Our <html> already reserves the scrollbar gutter (`scrollbar-gutter: stable` + `overflow-y: scroll`), so the compensation added a visible ~15px shift on every dropdown open. Pin body's padding-right with `!important` so the page stays still. (xuyuanwei678 review #2.) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The earlier placeholder approach didn't work — Radix's `placeholder` only fires for an empty `value`, not for an unmatched non-empty one. So an element with a legacy fontname (e.g. `Microsoft YaHei`, `PingFang SC`, theme defaults) outside the curated FONTS registry still rendered a blank trigger. Render the trigger text via `SelectValue` children instead — the new `currentFontLabel` helper covers all three cases: matched → entry's i18n / fallback label, unmatched non-empty → the raw family name, empty → the default-font label. Unit tests cover each case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ze work The onOpenChange handler added to silence the Radix dev warning + restore Esc dismissal also fired on pointer-down-outside — i.e. on every mousedown on the selected element to drag it or grab a resize handle. That cleared the selection before the drag could start, so nothing on the canvas could be moved or resized. preventDefault on `onPointerDownOutside` (matching the existing `onFocusOutside` hardening) keeps the bar selection-driven while leaving Esc as the legitimate onOpenChange path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the "auto-insert at a hidden default position" UX. Click `Text box` → arms text-insertion: the button takes the violet active style, and the renderer's existing ElementCreateSelection overlay turns the canvas cursor into a crosshair. On the canvas: - click → 300×60 box at the click point - drag → a box at the dragged rect Either way the new box is auto-selected (addElement defaults that on), and the surface's existing useEditingTextElementId picks it up so the AnchoredTextBar opens on it. Esc disarms; clicking the armed button again disarms (toggle). Completes the text branch in the renderer's `useInsertFromCreateSelection` (pptist scaffolding left it TODO) and bypasses the 200² square fallback in `ElementCreateSelection` for the text type (a square wouldn't suit a text box). `InsertPaletteItem` gains an `active?` field so `CommandBar` can render the armed style. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Tailwind's preflight resets `list-style` to none, so the format bar's `bulletList` toggle wrapped selected text in `<ul><li>` but no marker ever appeared — the button looked inert. Scope a list-style restoration to `.editable-element-text ul/ol/li` so bullets / numbers render in the slide text without leaking into the rest of the app. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The size was a read-only `<span>` between the −/+ steppers. Replace with an `<input type=text>` that mirrors `attrs.fontsize` locally, commits on Enter / blur (clamped to [8, 96]; non-numeric reverts), and reverts on Escape. Adds the `edit.text.fontSize` aria-label key in all 6 locales. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…icity) The earlier list CSS didn't survive Tailwind's preflight (which also resets `padding: 0` on `<ul>`/`<ol>`, so with `list-style-position: outside` the markers had no room to render). Add `!important` on `list-style` and `padding-inline-start`, and broaden to also match `.prosemirror-editor ul`/ `ol`/`li` in case the markup ever nests differently than expected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`richTextAttrs` is a single shared store updated by whichever ProseMirror was last focused. Switching from one text element to another visibly carried the previous element's toggle states (bold / italic / alignment / list) on the format bar for a moment — until the new element's ProseMirror took focus and repopulated the attrs. `useSyncEditingElementId` now resets the attrs to defaults whenever the editing id changes, so the bar shows a neutral state during the transition instead of stale. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…over Clicking the text-color swatch opened the browser's native `<input type=color>` dialog — off-brand and inconsistent across platforms. Swap it for a `ColorPicker` popover: a 12-swatch grid covering the common slide-text needs (4 neutrals + warm + cool) plus a hex input for anything else. Closes on pick. Selected swatch gets the violet outline; hex input commits on Enter / blur (reverts if invalid). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous popover was a chunky 12-swatch grid plus a hex input nobody types into. Rebuild on `react-colorful` (3KB, well-tested): - SV pad + hue slider for free-form picking, with scoped CSS overrides to keep the picker tight (128px pad height) and rounded — not stock. - OS eyedropper via the EyeDropper API, feature-detected (Chrome / Edge; hidden on Safari / Firefox). - Row of 10 small (18px) common colors at the foot for one-click reach. - Current-color preview + read-only hex display. - Hex input dropped entirely — picking is meant to be tactile. Live preview while dragging; the popover closes on a swatch / eyedropper commit (not on drag). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Each SV-pad / hue-slider drag tick fires onChange → dispatches the color command → `editorView.focus()` pulls focus out of the popover into ProseMirror. Radix's default onFocusOutside path was treating that as a dismiss, so the popover closed the instant a drag started — clicking anywhere on the picker shut it. preventDefault on `onFocusOutside` (mirrors the AnchoredBar hardening) keeps it open; the popover still closes on swatch / eyedropper commits and on outside-click / Esc. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…drag sync
Two follow-ups from a self-CR on the branch:
- `body { padding-right: 0 !important }` was global, overriding Radix's
`react-remove-scroll` compensation for every Dialog / Sheet / Select /
Popover across the app. Scope it to a `body[data-maic-editor='true']`
selector; `SlideCanvas` sets the attribute while mounted. Non-editor
pages get Radix's default behavior back.
- `ColorPicker`'s `useEffect(() => setColor(value), [value])` mirror
could race a stale `value` against the user's current pointer position
mid-drag — a single late round-trip would snap the picker back. Gate
the re-sync on `isDragging.current` (cleared on `pointerup`); external
commits (swatch / eyedropper) still sync immediately because they fire
while no drag is in flight.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Gate the `richTextAttrs` reset in `useSyncEditingElementId` to only fire on element-to-element transitions (track previous editing id via a ref). The unconditional reset on the first selection briefly flashed neutral defaults (color #000, fontsize 16px) before the focusing ProseMirror repopulated the real values. - Doc-comment the text-insertion add-element asymmetry: text uses the renderer's `addElement` (because the rect math lives there and we get auto-select for free), image uses surface-side `applyOp` (its source is the ImagePicker, not a canvas gesture). Both commit through the same store, but the text lane doesn't show as a typed `element.add` op in the session history — acceptable, now explicit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CR round-2 residual nit: the single `pointerup` listener that clears the drag-gate would silently keep the gate stuck on any browser / emulator that only emits the older mouse/touch families. Listen on all four (`mouseup`, `touchend`, `pointerup`, `pointercancel`) — belt-and-suspenders, no behavior change on the common path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`createDefaultImageElement` hardcoded the new image's box to 360×220, so anything not ~1.6:1 (which is almost everything users upload — photos, screenshots, logos) ended up squashed or stretched the moment it landed on the slide. Wrap the factory in `insertImageElement` that measures the source via `new Image()`, then dispatches `element.add` with dimensions scaled to fit MAX 600×400 while preserving the natural ratio. Load failure falls back to the factory default so insertion always succeeds. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`addElement` was only ever used by the inline image-insert which became `insertImageElement`; text uses `armText` (toggle). PPTElement-typed parameter was already unused after the text refactor — removing the dead helper resolves the lint warning. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rface After `addElement` was dropped (55a9a71), the `PPTElement` type import has no remaining consumers in this file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Follow-up to #586 — the UX polish explicitly deferred during PR2 review. Part of the slide surface track (#562) under #560.
What
The slide editor's element chrome was disjoint: a text-format popover pinned to the canvas top-center, a separate top-center toolbar for delete, a dashed select box stacked on the focused editor, a font picker that mostly didn't work, and
Text boxinsertion that dropped a box at a hardcoded(180, 140)with no visible feedback. This PR makes the editing experience selection-anchored and coherent.Selection-anchored bars — every selected element now gets a bar that hugs it (Figma/Pitch feel) and tracks it through drag / resize / zoom, instead of fixed toolbars at the canvas top. A text element gets the format controls + delete; every other element type (image, shape, line, table, …) gets a delete bar. The top-center
FloatingToolbaris retired for the slide surface.One clean editing frame — selecting a text element used to draw the dashed select frame and a UA focus ring on the editor. Editing a text element now shows a single clean solid frame (resize/rotate handles kept).
Modernized format bar — the bar used a native
<select>and a hand-rolled−/+stepper. Anchoring it made that prominent, so it's rebuilt with the design-systemSelect, an editable size input alongside the−/+stepper pill, a proper color picker (SV pad + hue slider + eyedropper + common-color strip), and unified control styling. Text commands unchanged. The color picker usesreact-colorful(~3 KB gz, MIT) for the SV/hue engine with scoped CSS overrides; everything else is hand-written.Real fonts — the picker listed 29 fonts but the app only ever loaded Inter; the other 28 had no
@font-face(a pre-existing gap — nothing but the format bar even importsconfigs/font.ts). A curated 14-font set now genuinely loads via@fontsource(npm-managed, no font binaries committed; CJK facesunicode-range-subsetted, so they download lazily): 思源黑体/宋体, 霞鹜文楷, 站酷快乐体 + 9 Latin families. Commercial decorative Chinese fonts are left out — they need self-hosting + a licensing review.Arm-and-place text insertion — clicking
Text boxin the toolbar arms text-insertion (button turns violet, canvas cursor becomes a crosshair). On the canvas, click drops a default-sized box at the click point; drag drops a box at the dragged rect. Either way the new box is auto-selected and the anchored bar opens on it. Esc disarms. Replaces the "auto-insert at a hidden default position" UX. Reuses the renderer's existingElementCreateSelectionoverlay +creatingElementstate — the text branch inuseInsertFromCreateSelectionwas a TODO that this PR completes.Also fixed along the way: a
font-familyquoting bug (spaced/numeric family names likeSource Sans 3threw ondocument.fonts.checkand silently failed to apply); the anchored frame masking the element's own move/text cursor (pointer-events); a stalerichTextAttrsflash when switching between text elements; list-marker visibility under Tailwind's preflight reset (scopedlist-style !importanton.editable-element-textlists); Radix'sreact-remove-scrollbody-padding compensation creating a layout shift on every dropdown open (editor-scopedbody[data-maic-editor] { padding-right: 0 !important }); a noisy "font is loading" toast.How
A single selected text element is the element being edited — the renderer already drops a caret on click, so there is no separate "selected-not-editing" state for text. The previously-unused
editingElementIdcanvas-store flag is the lever: the surface sets it, the renderer reads it.components/edit/surfaces/slide/) — additive:AnchoredBar(Radix Popover + a virtualPopoverAnchortracking the element's live screen rect viauseTrackedRect), theAnchoredTextBar/AnchoredDeleteBarcontent,editing-state.ts, theColorPicker, surface hooks, and the body-mode marker for the scoped padding override. Fonts load viaapp/editor-fonts.ts(imported once from the root layout).TextElementOperateswaps its dashed frame for a clean solid one wheneditingElementIdmatches (resize/rotate handles kept); thefontnamemark'stoDOMquotes the family name and rejects["\\];useInsertFromCreateSelectiongains its text branch (was a TODO from pptist scaffolding);ElementCreateSelectionhands raw start/end through for the text type instead of padding sub-30px gestures to a 200² square. The flag defaults to"", so the dashed frame is byte-unchanged for any consumer that never sets it; playback never rendersOperate. Plus one editor-scoped CSS rule removing the contenteditable focus ring. ProseMirror'seditablewiring is untouched.Not in scope
No two-stage (double-click) editing model. Per-type actions beyond delete for non-text elements (image replace / crop, etc.) are a later sub-PR.
Tests / verification
resolveSelectedElement,resolveEditingElementId), insert / delete helpers, and the text-format-bar's pure logic (stepFontSize,currentFontLabel).tsc/eslint/prettierclean; CI green.e2e/mock harness): bars anchor centered above the selected element, one clean solid frame replaces the dashed select chrome, the curated fonts render, arm-and-place text insertion lands at the gesture, and delete works. A committed editor e2e regression test, plus unit coverage for the new interactive components (useTrackedRect,AnchoredBar,ColorPicker, the arm-and-place text branch inuseInsertFromCreateSelection), is a sensible follow-up — they each have natural seams to test.