From d4eb9fdbd57d23b9af4f84ce5fe0051ff956acb5 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 03:16:33 -0400 Subject: [PATCH 1/2] feat(studio): edit and style text in the preview Double-press a text element in the canvas and the caret opens where you pressed, in the element itself rather than in a panel. Select characters and a small toolbar offers colour, bold, italic and underline, applied to exactly those characters. The toolbar lives in Studio's document rather than the composition's. Putting it in the preview would inject Studio's chrome into the user's composition, where a render would capture it and the composition's own styling would inherit into it. In a flex or grid container the rebuilt runs go inside one wrapper, so a coloured word cannot reflow the element it sits in. Also fixes the keyboard: the shortcut guards matched contenteditable=true only, so playback shortcuts ate letters typed into the composition. --- .../src/components/editor/DomEditOverlay.tsx | 53 ++- .../editor/DomEditSelectionChrome.test.tsx | 85 ++++ .../editor/DomEditSelectionChrome.tsx | 32 +- .../editor/InlineTextToolbar.test.tsx | 194 ++++++++ .../components/editor/InlineTextToolbar.tsx | 222 +++++++++ .../components/editor/OffCanvasIndicators.tsx | 25 + .../editor/domEditInlineText.test.ts | 73 +++ .../components/editor/domEditInlineText.ts | 93 ++++ .../editor/domEditInlineTextElement.test.ts | 59 +++ .../src/components/editor/domEditing.ts | 1 + .../src/components/editor/domEditingLayers.ts | 16 + .../editor/useInlineTextEditing.tsx | 112 +++++ .../studio/src/contexts/DomEditContext.tsx | 4 + .../studio/src/hooks/domEditPersistFailure.ts | 7 - .../src/hooks/useDomEditCommits.test.tsx | 64 ++- .../studio/src/hooks/useDomEditCommits.ts | 2 + .../studio/src/hooks/useDomEditSession.ts | 2 + .../studio/src/hooks/useDomEditTextCommits.ts | 106 ++++- .../src/hooks/useInlineTextEdit.test.tsx | 432 ++++++++++++++++++ .../studio/src/hooks/useInlineTextEdit.ts | 265 +++++++++++ .../src/player/lib/playbackShortcuts.ts | 9 +- packages/studio/src/utils/studioHelpers.ts | 7 +- .../studio/src/utils/typingTarget.test.ts | 47 ++ packages/studio/src/utils/typingTarget.ts | 39 ++ 24 files changed, 1881 insertions(+), 68 deletions(-) create mode 100644 packages/studio/src/components/editor/InlineTextToolbar.test.tsx create mode 100644 packages/studio/src/components/editor/InlineTextToolbar.tsx create mode 100644 packages/studio/src/components/editor/domEditInlineText.test.ts create mode 100644 packages/studio/src/components/editor/domEditInlineText.ts create mode 100644 packages/studio/src/components/editor/domEditInlineTextElement.test.ts create mode 100644 packages/studio/src/components/editor/useInlineTextEditing.tsx create mode 100644 packages/studio/src/hooks/useInlineTextEdit.test.tsx create mode 100644 packages/studio/src/hooks/useInlineTextEdit.ts create mode 100644 packages/studio/src/utils/typingTarget.test.ts create mode 100644 packages/studio/src/utils/typingTarget.ts diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 151c24ce08..11cd1f0d4c 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -16,7 +16,7 @@ import { resolveShiftClickCandidate, } from "./domEditOverlayGestures"; import { useDomEditOverlayRects } from "./useDomEditOverlayRects"; -import { OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; +import { ChildRectOutlines, OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; import { createDomEditOverlayGestureHandlers } from "./useDomEditOverlayGestures"; import { useDomEditNudge } from "./useDomEditNudge"; import { SnapGuideOverlay, type SnapGuidesState } from "./SnapGuideOverlay"; @@ -30,6 +30,7 @@ import { useDomEditCompositionRect } from "./useDomEditCompositionRect"; import { useMountEffect } from "../../hooks/useMountEffect"; import { startOffCanvasIndicatorRefresh } from "./offCanvasIndicatorRefresh"; import { CanvasContextMenu } from "./CanvasContextMenu"; +import { useInlineTextEditing } from "./useInlineTextEditing"; import type { ZOrderAction, ZOrderPatch } from "./canvasContextMenuZOrder"; import { getPreviewTargetFromPointer } from "../../utils/studioPreviewHelpers"; import { logSelect } from "../../utils/selectDebug"; @@ -164,6 +165,9 @@ export const DomEditOverlay = memo(function DomEditOverlay({ groupSelectionsRef.current = groupSelections; const hoverSelectionRef = useRef(hoverSelection); hoverSelectionRef.current = hoverSelection; + + // Double-click an element to edit its text where it sits. + const inlineText = useInlineTextEditing(selectionRef); const onPathOffsetCommitRef = useRef(onPathOffsetCommit); onPathOffsetCommitRef.current = onPathOffsetCommit; const onGroupPathOffsetCommitRef = useRef(onGroupPathOffsetCommit); @@ -370,6 +374,16 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return; } + // A second press on the same spot opens that element's text. This is the + // press path that actually runs: the pointer handler prevents the default + // on its way through, so the overlay's own mousedown never fires, and the + // browser never pairs the presses into a dblclick either. + if (inlineText.startFromPress(event)) { + event.preventDefault(); + event.stopPropagation(); + return; + } + const target = event.target as HTMLElement | null; if (target?.closest('[data-dom-edit-selection-box="true"]')) return; @@ -449,7 +463,12 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return (
{ + if (!inlineText.handleKeyDown(event)) return; + event.preventDefault(); + event.stopPropagation(); }} onPointerDown={handleOverlayPointerDown} onMouseDown={handleOverlayMouseDown} @@ -492,6 +519,7 @@ export const DomEditOverlay = memo(function DomEditOverlay({ )} {!hasGroupSelection && selection && overlayRect && compRect.width > 0 && ( )} - {childRects.length > 0 && - compRect.width > 0 && - childRects.map((cr, i) => ( -
- ))} + 0 ? childRects : []} /> + {/* Mounted here rather than with the selection chrome: the chrome does + not render for every selection, and the toolbar belongs to the + editing session, which does. */} + {inlineText.toolbar} { act(() => root.unmount()); }); }); + +// The bug: the overlay above the preview goes pointer-events-none while text is +// being edited, but `pointer-events: none` on a parent does not disable a child +// that sets `auto`. The selection box covers exactly the element being typed +// into, so it kept swallowing every press: the caret could only ever be placed +// once, when the edit opened, and dragging across characters did nothing. +describe("DomEditSelectionChrome while editing text", () => { + const CAPABLE = { + canCrop: true, + canApplyManualOffset: true, + canApplyManualSize: true, + canApplyManualRotation: true, + }; + + function renderChrome(editing: boolean) { + const element = document.createElement("div"); + element.id = "copy"; + document.body.append(element); + const selection = { + element, + id: "copy", + selector: "#copy", + capabilities: CAPABLE, + } as unknown as DomEditSelection; + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + return { host, unmount: () => act(() => root.unmount()) }; + } + + it("stops the selection box taking presses, so they reach the caret below", () => { + const { host, unmount } = renderChrome(true); + const box = host.querySelector('[data-dom-edit-selection-box="true"]')!; + expect(box.className).toContain("pointer-events-none"); + expect(box.className).not.toContain("pointer-events-auto"); + unmount(); + }); + + it("keeps the box interactive when no text is being edited", () => { + const { host, unmount } = renderChrome(false); + const box = host.querySelector('[data-dom-edit-selection-box="true"]')!; + expect(box.className).toContain("pointer-events-auto"); + unmount(); + }); + + it("still marks the edited element, so it is clear which one has the caret", () => { + const { host, unmount } = renderChrome(true); + const box = host.querySelector('[data-dom-edit-selection-box="true"]')!; + expect(box.className).toContain("border-studio-accent/80"); + unmount(); + }); + + it("takes away every handle that would sit over the text", () => { + const { host, unmount } = renderChrome(true); + expect(host.querySelectorAll(".pointer-events-auto")).toHaveLength(0); + expect(host.querySelector("[data-dom-edit-crop-frame]")).toBeNull(); + unmount(); + }); + + it("keeps the handles when nothing is being edited", () => { + const { host, unmount } = renderChrome(false); + expect(host.querySelectorAll(".pointer-events-auto").length).toBeGreaterThan(1); + unmount(); + }); +}); diff --git a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx index acbde93f69..8046e8608f 100644 --- a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx +++ b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx @@ -126,6 +126,12 @@ interface DomEditSelectionChromeProps { onStyleCommit?: (property: string, value: string) => Promise | void; onBoxMouseDown: (e: React.MouseEvent) => void; onBoxClick: (event: React.MouseEvent) => void; + /** The canvas' text-editing session: what opens one, and whether one is open. */ + inlineText?: { + editing: boolean; + /** Every press on the box. Returns true when it opened a text edit. */ + startFromPress: (event: React.PointerEvent) => boolean; + }; } // Oriented selection chrome: a rotation wrapper spanning the overlay, rotated by @@ -149,7 +155,16 @@ export function DomEditSelectionChrome({ onStyleCommit, onBoxMouseDown, onBoxClick, + inlineText, }: DomEditSelectionChromeProps) { + // While the text is being edited the chrome is a mark, not a control. The + // overlay above the preview already stands aside for the caret, but + // `pointer-events: none` on a parent does not disable a child that asks for + // them back, and the box is positioned to cover exactly the text being typed + // into: left interactive, it swallows every press, so the caret can never be + // moved and characters can never be selected by dragging. + const editing = inlineText?.editing ?? false; + return ( <>
- {allowCanvasMovement && selection.capabilities.canApplyManualRotation && ( + {allowCanvasMovement && !editing && selection.capabilities.canApplyManualRotation && ( { + // A second press opens the element's text for editing, and must be + // caught here rather than on the canvas: this handler prevents the + // default on the first press, which suppresses the compatibility + // mousedown the canvas would otherwise see, and the pointer capture + // it takes stops the browser pairing the presses into a dblclick. + if (inlineText?.startFromPress(e)) { + e.preventDefault(); + e.stopPropagation(); + return; + } if (!allowCanvasMovement || e.shiftKey) return; if (selection.capabilities.canApplyManualOffset) { gestures.startGesture("drag", e); @@ -221,6 +246,7 @@ export function DomEditSelectionChrome({ is positioned relative to the overlay container using the overlayRect origin, matching the old child-relative offsets. */} {allowCanvasMovement && + !editing && selection.capabilities.canApplyManualSize && RESIZE_HANDLE_DEFS.map((def) => def.handle !== "se" && !selection.capabilities.canApplyManualOffset ? null : ( @@ -245,7 +271,7 @@ export function DomEditSelectionChrome({
{/* Crop owns its element-local oriented frame. Keep it outside the chrome's rotated plane or a rotated selection applies the angle twice. */} - {selection.capabilities.canCrop && groupSelectionCount <= 1 && ( + {selection.capabilities.canCrop && !editing && groupSelectionCount <= 1 && ( { + document.body.innerHTML = ""; +}); + +/** An element standing in for one in the preview, and a fake frame around it. */ +function scene(html: string) { + document.body.innerHTML = `

${html}

`; + const element = document.body.firstElementChild as HTMLElement; + const iframe = document.createElement("iframe"); + document.body.append(iframe); + // The composition is drawn scaled, so the toolbar has to map out of it. + iframe.getBoundingClientRect = () => ({ left: 100, top: 50, width: 400 }) as DOMRect; + Object.defineProperty(iframe, "contentWindow", { value: window }); + const session: InlineTextEditSession = { element, original: html, outline: "" }; + return { element, iframe, session }; +} + +function render(session: InlineTextEditSession | null, iframe: HTMLIFrameElement | null) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => root.render()); + return { host, root, rerender: () => act(() => root.render(
)) }; +} + +function selectAll(element: HTMLElement) { + const range = document.createRange(); + range.selectNodeContents(element); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + act(() => void document.dispatchEvent(new Event("selectionchange"))); +} + +function toolbarIn(host: HTMLElement): HTMLElement | null { + return host.querySelector('[data-inline-text-toolbar="true"]'); +} + +describe("InlineTextToolbar", () => { + it("stays out of the way until characters are actually selected", () => { + const { session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + + expect(toolbarIn(host)).toBeNull(); + }); + + it("appears once a run of characters is selected", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + + selectAll(element); + + expect(toolbarIn(host)).not.toBeNull(); + }); + + it("goes away when the selection collapses again", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + const selection = document.getSelection()!; + act(() => { + selection.collapseToEnd(); + document.dispatchEvent(new Event("selectionchange")); + }); + + expect(toolbarIn(host)).toBeNull(); + }); + + it("shows nothing at all when no text is being edited", () => { + const { iframe } = scene("hello"); + const { host } = render(null, iframe); + + expect(toolbarIn(host)).toBeNull(); + }); + + it("keeps the press, so clicking a control does not collapse the selection", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + const press = new MouseEvent("mousedown", { bubbles: true, cancelable: true }); + act(() => void toolbarIn(host)!.dispatchEvent(press)); + + expect(press.defaultPrevented).toBe(true); + }); + + // It renders inside the canvas overlay, so a press it lets through is read as + // a click on the composition: the element deselects and the edit commits out + // from under the button that was just pressed. + it("keeps its presses away from the canvas underneath", () => { + const { element, session, iframe } = scene("hello world"); + const seen: string[] = []; + // A stand-in for the canvas overlay: the toolbar renders inside it, and + // these are the handlers that would deselect the element. + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => + root.render( +
seen.push("pointerdown")} + onMouseDown={() => seen.push("mousedown")} + onClick={() => seen.push("click")} + > + +
, + ), + ); + selectAll(element); + + const toolbar = toolbarIn(host)!; + act(() => { + for (const type of ["pointerdown", "mousedown", "click"]) { + toolbar.dispatchEvent(new MouseEvent(type, { bubbles: true, cancelable: true })); + } + }); + + expect(seen).toEqual([]); + }); + + // A colour input has a user-agent minimum width, so an invisible one pinned + // only by `inset-0` spills across its neighbours: hovering bold opened the + // colour picker. + it("keeps the invisible colour input inside its own swatch", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + const input = host.querySelector('input[type="color"]')!; + expect(input.className).toContain("w-full"); + expect(input.className).toContain("h-full"); + expect(input.className).toContain("min-w-0"); + }); + + it("styles the selected characters when a control is used", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + act(() => host.querySelector('[aria-label="Bold"]')!.click()); + + expect(element.innerHTML).toBe('hello world'); + }); + + it("reads back the styling it applied, so the control shows the truth", () => { + const { element, session, iframe } = scene('words'); + const { host } = render(session, iframe); + + selectAll(element); + + expect(host.querySelector('[aria-label="Italic"]')?.getAttribute("aria-pressed")).toBe("true"); + expect(host.querySelector('[aria-label="Bold"]')?.getAttribute("aria-pressed")).toBe("false"); + }); + + it("turns a style back off when the control is used again", () => { + const { element, session, iframe } = scene('words'); + const { host } = render(session, iframe); + selectAll(element); + + act(() => host.querySelector('[aria-label="Bold"]')!.click()); + + expect(element.innerHTML).toBe("words"); + }); + + it("places itself over the selection, mapped out of the scaled composition", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + const range = document.createRange(); + range.selectNodeContents(element); + range.getBoundingClientRect = () => + ({ left: 20, top: 40, width: 100, height: 10 }) as unknown as DOMRect; + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + act(() => void document.dispatchEvent(new Event("selectionchange"))); + + const toolbar = toolbarIn(host)!; + // Frame at 100,50; scale 400/innerWidth; centre of the range, above it. + const scale = 400 / window.innerWidth; + expect(toolbar.style.left).toBe(`${100 + (20 + 50) * scale}px`); + expect(toolbar.style.top).toBe(`${50 + 40 * scale - 10}px`); + }); +}); diff --git a/packages/studio/src/components/editor/InlineTextToolbar.tsx b/packages/studio/src/components/editor/InlineTextToolbar.tsx new file mode 100644 index 0000000000..8ecc2b43f7 --- /dev/null +++ b/packages/studio/src/components/editor/InlineTextToolbar.tsx @@ -0,0 +1,222 @@ +import { useCallback, useEffect, useState } from "react"; +import { applyInlineStyle, readInlineStyle } from "./inlineTextStyleRange"; +import type { InlineTextEditSession } from "../../hooks/useInlineTextEdit"; + +/** + * The controls for styling the characters selected inside an open text edit. + * + * It lives in Studio's document rather than the composition's, positioned over + * the selection: putting it in the preview would mean injecting Studio's chrome + * into the user's composition, where it would be captured by a render and + * inherit the composition's own styling. + * + * `position: fixed` and viewport coordinates, so it does not have to know which + * of the canvas' several nested coordinate systems it was mounted into. + */ + +const READ_PROPERTIES = ["color", "font-weight", "font-style", "text-decoration-line"]; + +/** Enough above the text to clear it, without leaving the element behind. */ +const GAP_PX = 10; +const DEFAULT_COLOR = "#ffffff"; + +interface ToolbarPlacement { + left: number; + top: number; + styles: Record; +} + +export function InlineTextToolbar({ + session, + iframe, +}: { + session: InlineTextEditSession | null; + iframe: HTMLIFrameElement | null; +}) { + const [placement, setPlacement] = useState(null); + + const refresh = useCallback(() => { + setPlacement(session && iframe ? placeOverSelection(session.element, iframe) : null); + }, [session, iframe]); + + // The selection lives in the preview's document, so the event does too. + useEffect(() => { + const doc = session?.element.ownerDocument; + if (!doc) { + setPlacement(null); + return; + } + doc.addEventListener("selectionchange", refresh); + return () => doc.removeEventListener("selectionchange", refresh); + }, [session, refresh]); + + const apply = useCallback( + (delta: Record) => { + const doc = session?.element.ownerDocument; + const range = doc?.defaultView?.getSelection()?.getRangeAt(0); + if (!range) return; + applyInlineStyle(range, delta); + refresh(); + }, + [session, refresh], + ); + + if (!placement) return null; + const styles = placement.styles; + + return ( +
event.stopPropagation()} + > + + apply({ "font-weight": on ? "700" : null })} + /> + apply({ "font-style": on ? "italic" : null })} + /> + apply({ "text-decoration-line": on ? "underline" : null })} + /> +
+ ); +} + +function swallow(event: { preventDefault: () => void; stopPropagation: () => void }): void { + event.preventDefault(); + event.stopPropagation(); +} + +function ToolbarToggle({ + label, + glyph, + on, + onToggle, + bold, + italic, + underline, +}: { + label: string; + glyph: string; + on: boolean; + onToggle: (on: boolean) => void; + bold?: boolean; + italic?: boolean; + underline?: boolean; +}) { + return ( + + ); +} + +/** Where the selection is on screen, or null when there is nothing selected. */ +function placeOverSelection( + element: HTMLElement, + iframe: HTMLIFrameElement, +): ToolbarPlacement | null { + const doc = element.ownerDocument; + const view = doc.defaultView; + const selection = view?.getSelection(); + if (!view || !selection || selection.rangeCount === 0 || selection.isCollapsed) return null; + + const range = selection.getRangeAt(0); + if (!element.contains(range.commonAncestorContainer)) return null; + const rect = range.getBoundingClientRect(); + + // The composition is drawn scaled into the iframe's box, so a point inside it + // is that scale away from a point on Studio's screen. This is the inverse of + // the mapping the canvas uses to turn a press into a caret position. + const box = iframe.getBoundingClientRect(); + const scale = view.innerWidth ? box.width / view.innerWidth : 1; + + return { + left: box.left + (rect.left + rect.width / 2) * scale, + top: box.top + rect.top * scale - GAP_PX, + styles: readInlineStyle(range, READ_PROPERTIES), + }; +} + +function isBold(weight: string | undefined): boolean { + if (!weight) return false; + if (weight === "bold" || weight === "bolder") return true; + return Number.parseInt(weight, 10) >= 600; +} + +/** + * A colour input only accepts `#rrggbb`, and what the page reports is whatever + * the stylesheet said. An unreadable value opens the picker on white rather + * than refusing to open. + */ +function toHexColor(value: string | undefined): string { + if (!value) return DEFAULT_COLOR; + if (/^#[0-9a-f]{6}$/i.test(value)) return value; + const channels = value.match(/\d+(\.\d+)?/g); + if (!channels || channels.length < 3) return DEFAULT_COLOR; + return `#${channels + .slice(0, 3) + .map((channel) => Number(channel).toString(16).padStart(2, "0")) + .join("")}`; +} diff --git a/packages/studio/src/components/editor/OffCanvasIndicators.tsx b/packages/studio/src/components/editor/OffCanvasIndicators.tsx index ead1a53a4f..fe0edcf8a5 100644 --- a/packages/studio/src/components/editor/OffCanvasIndicators.tsx +++ b/packages/studio/src/components/editor/OffCanvasIndicators.tsx @@ -143,3 +143,28 @@ export function OffCanvasIndicators({ ); } + +/** + * The dashed outlines around a selected element's children. + * + * Extracted from the canvas overlay, which is at its 600-line limit, and it + * sits here rather than in its own file because it is the same kind of thing: + * a passive, non-interactive mark the overlay draws over the composition. + */ +export function ChildRectOutlines({ + rects, +}: { + rects: ReadonlyArray<{ left: number; top: number; width: number; height: number }>; +}) { + return ( + <> + {rects.map((rect, index) => ( +
+ ))} + + ); +} diff --git a/packages/studio/src/components/editor/domEditInlineText.test.ts b/packages/studio/src/components/editor/domEditInlineText.test.ts new file mode 100644 index 0000000000..a229e8cd49 --- /dev/null +++ b/packages/studio/src/components/editor/domEditInlineText.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it, vi } from "vitest"; +import type { DomEditSelection, DomEditTextField } from "./domEditingTypes"; + +const editable = vi.hoisted(() => ({ current: true })); +vi.mock("./domEditingLayers", () => ({ + isTextEditableSelection: () => editable.current, +})); + +const { canEditTextInline } = await import("./domEditInlineText"); + +function field(key: string): DomEditTextField { + return { + key, + label: key, + value: "text", + tagName: "SPAN", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }; +} + +function selection(partial: Partial = {}): DomEditSelection { + return { + label: "Heading", + tagName: "H1", + isCompositionHost: false, + isInsideLockedComposition: false, + textFields: [field("self")], + ...partial, + } as DomEditSelection; +} + +describe("canEditTextInline", () => { + it("allows an element the panel would let you edit text on", () => { + editable.current = true; + expect(canEditTextInline(selection())).toBe(true); + }); + + // The bar is the panel's bar: nothing becomes editable here that is not + // editable there. + it("refuses an element whose text the panel cannot edit either", () => { + editable.current = false; + expect(canEditTextInline(selection())).toBe(false); + }); + + // Editing the whole element would flatten its children into one string. + it("refuses an element with several text fields", () => { + editable.current = true; + expect(canEditTextInline(selection({ textFields: [field("a"), field("b")] }))).toBe(false); + }); + + it("allows an element with no separate text fields", () => { + editable.current = true; + expect(canEditTextInline(selection({ textFields: [] }))).toBe(true); + }); + + it("refuses the composition host, which is the document rather than copy", () => { + editable.current = true; + expect(canEditTextInline(selection({ isCompositionHost: true }))).toBe(false); + }); + + it("refuses anything inside a locked composition", () => { + editable.current = true; + expect(canEditTextInline(selection({ isInsideLockedComposition: true }))).toBe(false); + }); + + it("refuses nothing at all", () => { + editable.current = true; + expect(canEditTextInline(null)).toBe(false); + }); +}); diff --git a/packages/studio/src/components/editor/domEditInlineText.ts b/packages/studio/src/components/editor/domEditInlineText.ts new file mode 100644 index 0000000000..451c5b1832 --- /dev/null +++ b/packages/studio/src/components/editor/domEditInlineText.ts @@ -0,0 +1,93 @@ +import { isRichTextFormattingTag } from "@hyperframes/core/rich-text-sanitize"; +import type { DomEditSelection } from "./domEditingTypes"; +import { isTextEditableSelection } from "./domEditingLayers"; + +/** + * Whether this element's text can be edited where it sits. + * + * Its own function rather than a condition inside a handler, because this is + * the rule most likely to change: it is the whole answer to "why did nothing + * happen when I double-clicked that". + * + * The bar is deliberately the same as the design panel's, plus one thing the + * panel can do that editing in place cannot. An element with several text + * fields is edited a field at a time there, and making the whole element + * editable would flatten its children into one string, so those keep the panel. + */ +export function canEditTextInline(selection: DomEditSelection | null): boolean { + if (!selection) return false; + if (!isTextEditableSelection(selection)) return false; + // The composition host is the document, not a piece of copy in it. + if (selection.isCompositionHost) return false; + if (selection.isInsideLockedComposition) return false; + if (selection.textFields.length <= 1) return true; + // A styled element reports one field per run of characters, but it is still + // one piece of copy and the caret edits all of it at once. + return canEditElementTextInline(selection.element); +} + +/** + * Whether this element's text can be edited in place, judged from the element + * alone. + * + * The press path cannot use the selection-shaped gate above: building a + * selection is asynchronous, and a press has to decide now whether it is a + * text edit or the start of a drag. This asks the same question of the DOM. + * + * A structural child keeps an element out: those are separate text fields, the + * panel edits them one at a time, and making the whole element editable would + * flatten them into a single string. + * + * A formatting child does not. Styling a run of characters puts a span inside + * the element, so a rule of "no element children" would have let the editor + * lock every element it had ever styled out of itself, permanently, on the + * first colour change. What counts as formatting is the sanitiser's allowlist, + * so the editor and the thing that writes the file agree on it. + */ +export function canEditElementTextInline(element: HTMLElement | null): boolean { + if (!element) return false; + const tag = element.tagName; + if (tag === "BODY" || tag === "HTML") return false; + if (!hasOnlyFormattingChildren(element)) return false; + if (element.isContentEditable) return false; + return (element.textContent ?? "").trim().length > 0; +} + +function hasOnlyFormattingChildren(element: HTMLElement): boolean { + for (const child of Array.from(element.children)) { + if (!isRichTextFormattingTag(child.tagName)) return false; + // Formatting nests, and a structural child hidden inside a span is still + // structural. + if (!hasOnlyFormattingChildren(child as HTMLElement)) return false; + } + return true; +} + +/** Where and when a press landed, for recognising the next one as a pair. */ +export interface PressMark { + x: number; + y: number; + at: number; +} + +/** Long enough to be deliberate, short enough not to catch two separate clicks. */ +const DOUBLE_PRESS_MS = 450; +/** A double press is two presses in the same place, not a tiny drag. */ +const DOUBLE_PRESS_SLOP_PX = 6; + +/** + * Whether this press pairs with the last one into a double press. + * + * Studio cannot use `dblclick` or a click count for this. The selection box + * takes pointer capture on the first press and prevents its default, which + * suppresses the compatibility mouse events and stops the browser pairing the + * two presses at all: no `dblclick` is dispatched, and `detail` stays 1. + */ +export function isDoublePress(previous: PressMark | null, next: PressMark): boolean { + if (!previous) return false; + return ( + next.at - previous.at <= DOUBLE_PRESS_MS && + Math.abs(next.x - previous.x) <= DOUBLE_PRESS_SLOP_PX && + Math.abs(next.y - previous.y) <= DOUBLE_PRESS_SLOP_PX + ); +} diff --git a/packages/studio/src/components/editor/domEditInlineTextElement.test.ts b/packages/studio/src/components/editor/domEditInlineTextElement.test.ts new file mode 100644 index 0000000000..c43ab7c807 --- /dev/null +++ b/packages/studio/src/components/editor/domEditInlineTextElement.test.ts @@ -0,0 +1,59 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it } from "vitest"; +import { canEditElementTextInline } from "./domEditInlineText"; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function mount(html: string): HTMLElement { + document.body.innerHTML = html; + return document.body.firstElementChild as HTMLElement; +} + +describe("canEditElementTextInline", () => { + it("opens a plain piece of copy", () => { + expect(canEditElementTextInline(mount("

hello world

"))).toBe(true); + }); + + // The trap this exists for: styling a run of characters puts a span inside + // the element, and a rule of "no element children" would have let the editor + // lock every element it had ever styled out of itself, permanently. + it("still opens an element that has been styled", () => { + const element = mount('

hello world

'); + expect(canEditElementTextInline(element)).toBe(true); + }); + + it("opens an element whose formatting is nested", () => { + const element = mount('

deep

'); + expect(canEditElementTextInline(element)).toBe(true); + }); + + it("keeps out an element with a structural child, which the panel edits field by field", () => { + expect(canEditElementTextInline(mount("

a

b

"))).toBe(false); + }); + + it("keeps out an element hiding something structural inside its formatting", () => { + expect(canEditElementTextInline(mount("

a

"))).toBe(false); + }); + + it("keeps out the document itself", () => { + expect(canEditElementTextInline(document.body)).toBe(false); + expect(canEditElementTextInline(document.documentElement)).toBe(false); + }); + + it("keeps out an element that is already being edited", () => { + const element = mount("

hello

"); + element.setAttribute("contenteditable", "true"); + expect(canEditElementTextInline(element)).toBe(false); + }); + + it("keeps out an element with no words in it", () => { + expect(canEditElementTextInline(mount("

"))).toBe(false); + }); + + it("keeps out nothing at all", () => { + expect(canEditElementTextInline(null)).toBe(false); + }); +}); diff --git a/packages/studio/src/components/editor/domEditing.ts b/packages/studio/src/components/editor/domEditing.ts index 1dd6a1ffa7..7934c87078 100644 --- a/packages/studio/src/components/editor/domEditing.ts +++ b/packages/studio/src/components/editor/domEditing.ts @@ -27,6 +27,7 @@ export { export { buildDefaultDomEditTextField, buildDomEditPatchTarget, + buildDomEditRichTextPatchOperation, buildDomEditStylePatchOperation, buildDomEditTextPatchOperation, collectDomEditLayerItems, diff --git a/packages/studio/src/components/editor/domEditingLayers.ts b/packages/studio/src/components/editor/domEditingLayers.ts index 3102d030f8..53bd536c65 100644 --- a/packages/studio/src/components/editor/domEditingLayers.ts +++ b/packages/studio/src/components/editor/domEditingLayers.ts @@ -527,6 +527,22 @@ export function buildDomEditTextPatchOperation( }; } +/** + * Replace an element's whole contents with markup, for a change no per-child + * operation can express — a text layer added, removed or reordered. + * + * The same operation an inline style edit uses, because it is the same thing: + * this element's children are now these. Both ends run the markup through the + * one sanitizer before it reaches a file. + */ +export function buildDomEditRichTextPatchOperation(value: string): PatchOperation { + return { + type: "rich-text", + property: "", + value, + }; +} + // ─── Non-editable reason ───────────────────────────────────────────────────── function hasSupportedDirectEdit(capabilities: DomEditCapabilities): boolean { diff --git a/packages/studio/src/components/editor/useInlineTextEditing.tsx b/packages/studio/src/components/editor/useInlineTextEditing.tsx new file mode 100644 index 0000000000..9c137cbb65 --- /dev/null +++ b/packages/studio/src/components/editor/useInlineTextEditing.tsx @@ -0,0 +1,112 @@ +import { useRef, type ReactNode, type RefObject } from "react"; +import { InlineTextToolbar } from "./InlineTextToolbar"; +import { useDomEditActionsContextOptional } from "../../contexts/DomEditContext"; +import { useInlineTextEdit } from "../../hooks/useInlineTextEdit"; +import { usePlayerStore } from "../../player/store/playerStore"; +import { getPreviewTargetFromPointer } from "../../utils/studioPreviewHelpers"; +import { + canEditElementTextInline, + canEditTextInline, + isDoublePress, + type PressMark, +} from "./domEditInlineText"; +import type { DomEditSelection } from "./domEditingTypes"; + +/** + * The canvas' side of editing text where it sits. + * + * Holds the session, decides which presses open one, and knows the two things + * the canvas has to do differently while one is open: stand aside so the caret + * underneath can be reached, and stop taking focus back. + * + * The actions context is read here rather than threaded through the overlay's + * props, the same way the agent surfaces in that overlay read it, and it is + * absent in standalone player mounts, which have no project to edit. + */ +export function useInlineTextEditing(selectionRef: RefObject): { + editing: boolean; + /** Open an edit when this press pairs with the last one. */ + startFromPress: (event: { clientX: number; clientY: number }) => boolean; + /** Handle a key on the canvas. Returns true when it opened an edit. */ + handleKeyDown: (event: { key: string; shiftKey: boolean }) => boolean; + /** + * The styling controls for the current selection, for the caller to render. + * + * Handed back rather than mounted somewhere central because it belongs to the + * session this hook owns, and appears and disappears with it. + */ + toolbar: ReactNode; +} { + const actions = useDomEditActionsContextOptional(); + const inlineText = useInlineTextEdit({ + onCommit: (html) => void actions?.handleDomRichTextCommit(html), + onPause: () => usePlayerStore.getState().setIsPlaying(false), + }); + const lastPressRef = useRef(null); + + /** + * The element under this press, hit-tested now rather than read from state. + * + * Every cached answer in the overlay is React state that has not caught up + * with the press happening now: the hover is documented as an async cache, + * and the selection from the first press has not re-rendered. Reading either + * meant the first double press on any element opened nothing, which is every + * double press that matters. + */ + const elementUnderPress = (event: { clientX: number; clientY: number }) => { + const iframe = actions?.previewIframeRef?.current; + if (!iframe) return null; + // Studio suppresses pointer events inside the composition so the canvas + // overlay can own input, which means a plain elementFromPoint only ever + // finds wrappers. This helper lifts that for the length of the hit test, + // and is the same one the canvas uses to decide what was clicked. + return getPreviewTargetFromPointer( + iframe, + event.clientX, + event.clientY, + selectionRef.current?.compositionPath ?? null, + ); + }; + + /** A point on Studio's canvas, in the scaled composition's coordinates. */ + const compositionPoint = (event: { clientX: number; clientY: number }) => { + const iframe = actions?.previewIframeRef?.current; + const view = iframe?.contentWindow; + if (!iframe || !view?.innerWidth) return undefined; + const box = iframe.getBoundingClientRect(); + const scale = box.width / view.innerWidth || 1; + return { x: (event.clientX - box.left) / scale, y: (event.clientY - box.top) / scale }; + }; + + return { + editing: inlineText.session !== null, + toolbar: ( + + ), + // Enter opens the selected element's text, the way every design tool does, + // and is the dependable way in: a double press has to survive the canvas' + // gesture machinery, while this is one key on a selection that has settled. + handleKeyDown: (event) => { + if (event.key !== "Enter" || event.shiftKey) return false; + const target = selectionRef.current; + if (inlineText.session || !canEditTextInline(target)) return false; + return inlineText.start(target!.element); + }, + startFromPress: (event) => { + const press = { x: event.clientX, y: event.clientY, at: Date.now() }; + const paired = isDoublePress(lastPressRef.current, press); + lastPressRef.current = press; + + if (!paired || inlineText.session) return false; + + const element = elementUnderPress(event); + if (!canEditElementTextInline(element)) return false; + // The caret opens where the press landed, which means mapping the point + // out of Studio's coordinates and into the composition's own. + return inlineText.start(element!, compositionPoint(event)); + }, + }; +} diff --git a/packages/studio/src/contexts/DomEditContext.tsx b/packages/studio/src/contexts/DomEditContext.tsx index f89e27e915..ff810b1e53 100644 --- a/packages/studio/src/contexts/DomEditContext.tsx +++ b/packages/studio/src/contexts/DomEditContext.tsx @@ -24,6 +24,7 @@ export interface DomEditActionsValue extends Pick< | "handleDomRotationCommit" | "handleDomManualEditsReset" | "handleDomTextCommit" + | "handleDomRichTextCommit" | "handleDomTextFieldStyleCommit" | "handleDomAddTextField" | "handleDomRemoveTextField" @@ -149,6 +150,7 @@ export function DomEditProvider({ handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, @@ -236,6 +238,7 @@ export function DomEditProvider({ handleDomRotationCommit, handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, @@ -305,6 +308,7 @@ export function DomEditProvider({ handleDomRotationCommit, handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, diff --git a/packages/studio/src/hooks/domEditPersistFailure.ts b/packages/studio/src/hooks/domEditPersistFailure.ts index 1e7bd7a406..7e04e6f75b 100644 --- a/packages/studio/src/hooks/domEditPersistFailure.ts +++ b/packages/studio/src/hooks/domEditPersistFailure.ts @@ -19,13 +19,6 @@ export class DomEditPersistUnsafeValueError extends Error { } } -export class DomEditPersistUnsupportedTextStructureError extends Error { - constructor() { - super("Couldn't save this text structure change"); - this.name = "DomEditPersistUnsupportedTextStructureError"; - } -} - export type DomEditPersistFailureSelection = Pick< DomEditSelection, "label" | "hfId" | "id" | "selector" | "selectorIndex" | "sourceFile" diff --git a/packages/studio/src/hooks/useDomEditCommits.test.tsx b/packages/studio/src/hooks/useDomEditCommits.test.tsx index 1f0af1c06e..102b9c8c68 100644 --- a/packages/studio/src/hooks/useDomEditCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditCommits.test.tsx @@ -117,14 +117,6 @@ function stubPatchFetch( return fetchMock; } -function stubUnexpectedPersistFetch() { - const fetchMock = vi.fn(async (): Promise => { - throw new Error("persist should not run"); - }); - vi.stubGlobal("fetch", fetchMock); - return fetchMock; -} - async function flushAsyncWork(): Promise { for (let i = 0; i < 8; i += 1) { await Promise.resolve(); @@ -812,14 +804,26 @@ function renderStyleCommitWithFetch(fetchHandler: FetchHandler) { }; } -async function expectRejectedTextStructureEdit( +/** + * Adding or removing a text layer, which no per-child operation can express. + * + * Both used to be refused outright — the panel offered the buttons and neither + * could ever save — so this asserts the opposite of what it used to: one + * `rich-text` operation carrying the element's new markup, and no complaint. + */ +async function expectPersistedTextStructureEdit( commit: (hook: ReturnType) => Promise, + expectedMarkup: (markup: string) => void, ): Promise { - const fetchMock = stubUnexpectedPersistFetch(); + const fetchMock = stubPatchFetch({ + ok: true, + changed: true, + matched: true, + content: '
First
', + }); const { iframe, element } = createPreviewElement( '
FirstSecond
', ); - const originalInnerHtml = element.innerHTML; const selection = createSelection(element, { textFields: [ textField({ key: "first", value: "First", source: "child" }), @@ -833,13 +837,17 @@ async function expectRejectedTextStructureEdit( await commit(rendered.hook); }); - expect(fetchMock).not.toHaveBeenCalled(); - expect(rendered.showToast).toHaveBeenCalledWith( - expect.stringContaining("text structure change"), - "error", + const patchPost = fetchMock.mock.calls.find((call) => + requestUrl(call[0]).includes("/file-mutations/patch-element/"), ); - expect(element.innerHTML).toBe(originalInnerHtml); - expect(rendered.recordEdit).not.toHaveBeenCalled(); + expect(patchPost).toBeDefined(); + const body = JSON.parse(String(patchPost?.[1]?.body)) as { + operations: Array<{ type: string; value?: string }>; + }; + expect(body.operations).toHaveLength(1); + expect(body.operations[0]?.type).toBe("rich-text"); + expectedMarkup(body.operations[0]?.value ?? ""); + expect(rendered.showToast).not.toHaveBeenCalled(); } finally { rendered.cleanup(); } @@ -1146,12 +1154,26 @@ describe("useDomEditCommits style persist handling", () => { } }); - it("refuses added child text fields without persisting serialized markup", async () => { - await expectRejectedTextStructureEdit((hook) => hook.handleDomAddTextField("first")); + it("persists an added child text field as the element's new markup", async () => { + await expectPersistedTextStructureEdit( + (hook) => hook.handleDomAddTextField("first"), + (markup) => { + expect(markup).toContain("First"); + expect(markup).toContain("Second"); + // The layer that was added, between the two that were there. + expect(markup.match(/ { - await expectRejectedTextStructureEdit((hook) => hook.handleDomRemoveTextField("first")); + it("persists a removed child text field as the element's new markup", async () => { + await expectPersistedTextStructureEdit( + (hook) => hook.handleDomRemoveTextField("first"), + (markup) => { + expect(markup).not.toContain("First"); + expect(markup).toContain("Second"); + }, + ); }); it("keeps single self text commits on the text-content path", async () => { diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index deb5ae9e3c..55e4125bae 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -378,6 +378,7 @@ export function useDomEditCommits({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, commitDomTextFields, handleDomTextFieldStyleCommit, handleDomAddTextField, @@ -441,6 +442,7 @@ export function useDomEditCommits({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, commitDomTextFields, handleDomTextFieldStyleCommit, handleDomAddTextField, diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index 5db396307d..5b11e467a7 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -229,6 +229,7 @@ export function useDomEditSession({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, @@ -499,6 +500,7 @@ export function useDomEditSession({ handleDomRotationCommit: handleGsapAwareRotationCommit, handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index 7111356279..dda80f816f 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -11,6 +11,7 @@ import { ensureImportedFontFace, } from "../utils/studioFontHelpers"; import { + buildDomEditRichTextPatchOperation, buildDomEditStylePatchOperation, buildDomEditTextPatchOperation, findElementForSelection, @@ -23,11 +24,9 @@ import { } from "../components/editor/domEditing"; import type { ImportedFontAsset } from "../components/editor/fontAssets"; import type { PersistDomEditOperations } from "./domEditCommitTypes"; +import { canEditElementTextInline } from "../components/editor/domEditInlineText"; import { buildTextFieldChildOperations } from "./domEditTextFieldCommitOps"; -import { - DomEditPersistUnsupportedTextStructureError, - reportDomEditPersistFailure, -} from "./domEditPersistFailure"; +import { reportDomEditPersistFailure } from "./domEditPersistFailure"; import { bumpDomEditCommitMapVersion, bumpDomEditCommitVersion, @@ -102,9 +101,20 @@ function planDomTextCommit( const childOperations = usesSerializedTextFields ? buildTextFieldChildOperations(originalTextFields, nextTextFields) : null; + // Per-child operations when the layers still line up one-for-one, and the + // element's whole markup when they do not. + // + // `buildTextFieldChildOperations` can only address children that already + // exist, so it returns null for any change in how many there are — which is + // every delete and every add. That used to end here with "Couldn't save this + // text structure change": the panel offered a remove button and an Add text + // field row, and neither could ever save. A structure change has one honest + // operation, which is to write the structure. const operations = childOperations ?? - (usesSerializedTextFields ? [] : [buildDomEditTextPatchOperation(nextContent)]); + (usesSerializedTextFields + ? [buildDomEditRichTextPatchOperation(nextContent)] + : [buildDomEditTextPatchOperation(nextContent)]); return { usesSerializedTextFields, @@ -269,9 +279,6 @@ export function useDomEditTextCommits({ } }, persist: async () => { - if (textCommit.usesSerializedTextFields && textCommit.childOperations === null) { - throw new DomEditPersistUnsupportedTextStructureError(); - } await persistDomEditOperations(domEditSelection, textCommit.operations, { label: "Edit text", skipRefresh: true, @@ -307,6 +314,85 @@ export function useDomEditTextCommits({ ], ); + /** + * Persist an element's own markup, for a text edit that styled part of it. + * + * Its own commit rather than a mode of the one above: that one plans a change + * to the text-field model, which escapes markup on the way out and refuses a + * change in child structure, and both of those are correct for the design + * panel. Styling a run of characters is neither of those things. The element + * already holds what the user typed, so there is nothing to apply, only + * something to save and something to put back if saving fails. + */ + const handleDomRichTextCommit = useCallback( + async (html: string) => { + if (!domEditSelection) return; + // The same gate that let the edit open, not the design panel's. + // + // The panel's rule is about its text fields, and it has none for an + // element whose text contains a line break: a `` holding `
`s + // is not a leaf, so nothing inside is a field and the element reports no + // editable text at all. Editing in place does not use fields — it + // rewrites the element's own markup — so refusing on that rule refused + // elements the caret had just been opened in, and every colour the user + // chose was dropped on the way out with nothing said about it. + if (!canEditElementTextInline(domEditSelection.element)) return; + const isLatestTextCommit = bumpDomEditCommitVersion(domTextCommitVersionRef); + const operations: PatchOperation[] = [{ type: "rich-text", property: "", value: html }]; + const iframe = previewIframeRef.current; + const doc = iframe?.contentDocument; + let editedElement: HTMLElement | null = null; + let previousInnerHtml: string | null = null; + + await runDomEditCommit({ + capture: () => { + if (!doc) return; + const el = findElementForSelection(doc, domEditSelection, activeCompPath); + if (!el) return; + editedElement = el; + previousInnerHtml = el.innerHTML; + }, + apply: () => { + // Idempotent: the caret put this there. Assigned anyway so a commit + // raised from anywhere but the element itself still lands. + if (editedElement) editedElement.innerHTML = html; + }, + persist: async () => { + await persistDomEditOperations(domEditSelection, operations, { + label: "Edit text", + skipRefresh: true, + shouldSave: isLatestTextCommit, + }); + }, + shouldRevert: () => isLatestTextCommit(), + revert: () => { + if (!editedElement || previousInnerHtml === null) return; + editedElement.innerHTML = previousInnerHtml; + }, + onError: (error) => + reportDomEditPersistFailure(domEditSelection, operations, error, showToast), + shouldResync: isLatestTextCommit, + resync: () => + resyncDomTextSelectionFromPreview( + doc, + domEditSelection, + activeCompPath, + buildDomSelectionFromTarget, + applyDomSelection, + ), + }); + }, + [ + activeCompPath, + applyDomSelection, + buildDomSelectionFromTarget, + domEditSelection, + persistDomEditOperations, + previewIframeRef, + showToast, + ], + ); + const commitDomTextFields = useCallback( async ( selection: DomEditSelection, @@ -342,9 +428,6 @@ export function useDomEditTextCommits({ } }, persist: async () => { - if (textCommit.usesSerializedTextFields && textCommit.childOperations === null) { - throw new DomEditPersistUnsupportedTextStructureError(); - } await persistDomEditOperations(selection, textCommit.operations, { label: "Edit text", skipRefresh: true, @@ -477,6 +560,7 @@ export function useDomEditTextCommits({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, commitDomTextFields, handleDomTextFieldStyleCommit, handleDomAddTextField, diff --git a/packages/studio/src/hooks/useInlineTextEdit.test.tsx b/packages/studio/src/hooks/useInlineTextEdit.test.tsx new file mode 100644 index 0000000000..125980fd99 --- /dev/null +++ b/packages/studio/src/hooks/useInlineTextEdit.test.tsx @@ -0,0 +1,432 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useInlineTextEdit, type InlineTextEditControls } from "./useInlineTextEdit"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +/** A heading in the document, standing in for one in the preview. */ +function heading(text = "Motion Playground"): HTMLElement { + const element = document.createElement("h1"); + element.textContent = text; + document.body.append(element); + return element; +} + +function mount(onCommit = vi.fn(), onPause = vi.fn()) { + const controls: { current: InlineTextEditControls | null } = { current: null }; + function Probe() { + controls.current = useInlineTextEdit({ onCommit, onPause }); + return null; + } + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => root.render()); + return { controls: () => controls.current!, root, onCommit, onPause }; +} + +describe("useInlineTextEdit", () => { + // Selecting the whole text would mean the next keystroke destroys it, which + // is a bad thing to do to someone who double-clicked to fix a typo. + it("leaves the caret after the last character, with nothing selected", async () => { + const element = heading("Motion Playground"); + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + + const selection = document.getSelection()!; + expect(selection.toString()).toBe(""); + expect(selection.isCollapsed).toBe(true); + expect(selection.anchorOffset).toBe(element.textContent!.length); + act(() => root.unmount()); + }); + + it("makes the element editable, and focuses it once the press has finished", async () => { + const element = heading(); + const { controls, root, onPause } = mount(); + + act(() => { + controls().start(element); + }); + + // Not `plaintext-only`: that would make it impossible to give three + // characters a colour, which is the point of editing in the composition. + expect(element.getAttribute("contenteditable")).toBe("true"); + expect(controls().session?.element).toBe(element); + // The frame being edited is the one the user chose to edit on. + expect(onPause).toHaveBeenCalledTimes(1); + + // Focus lands on the next frame, after the press that opened this and the + // click that follows it have both been and gone. + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + expect(document.activeElement).toBe(element); + act(() => root.unmount()); + }); + + it("hands the current text over exactly once when it commits", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + + act(() => { + controls().start(element); + }); + element.textContent = "Motion Playground Live"; + act(() => controls().commit()); + + expect(onCommit.mock.calls).toEqual([["Motion Playground Live"]]); + act(() => root.unmount()); + }); + + // Cancelling must leave the preview exactly as it found it: the element was + // being mutated live, and nothing was persisted. + it("puts the original text back on cancel, and commits nothing", () => { + const element = heading("Motion Playground"); + const { controls, root, onCommit } = mount(); + + act(() => { + controls().start(element); + }); + element.textContent = "half-typed"; + act(() => controls().cancel()); + + expect(element.textContent).toBe("Motion Playground"); + expect(onCommit).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it.each([ + ["commit", (c: InlineTextEditControls) => c.commit()], + ["cancel", (c: InlineTextEditControls) => c.cancel()], + ])("stops being editable after %s", (_name, close) => { + const element = heading(); + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + act(() => close(controls())); + + expect(element.hasAttribute("contenteditable")).toBe(false); + expect(controls().session).toBeNull(); + act(() => root.unmount()); + }); + + // A session that failed to close would leave the canvas unable to select. + it("tears down once when it is closed twice", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + + act(() => { + controls().start(element); + }); + act(() => controls().commit()); + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + }); + + it("refuses to open a second session over an open one", () => { + const first = heading("first"); + const second = heading("second"); + const { controls, root } = mount(); + + let opened = false; + act(() => { + controls().start(first); + opened = controls().start(second); + }); + + expect(opened).toBe(false); + expect(controls().session?.element).toBe(first); + expect(second.hasAttribute("contenteditable")).toBe(false); + act(() => root.unmount()); + }); + + // The composition reloads while an edit is open often enough to matter. + it("closes without throwing when the element has left the document", () => { + const element = heading(); + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + element.remove(); + + expect(() => act(() => controls().cancel())).not.toThrow(); + expect(controls().session).toBeNull(); + act(() => root.unmount()); + }); + + describe("the keys that end it", () => { + function press(element: HTMLElement, key: string, shiftKey = false) { + act(() => { + element.dispatchEvent(new KeyboardEvent("keydown", { key, shiftKey, bubbles: true })); + }); + } + + it("commits on Enter", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + element.textContent = "Renamed"; + press(element, "Enter"); + + expect(onCommit.mock.calls).toEqual([["Renamed"]]); + expect(controls().session).toBeNull(); + act(() => root.unmount()); + }); + + // A multi-line element still needs a way to get a line break. + it("leaves Shift+Enter alone", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + press(element, "Enter", true); + + expect(onCommit).not.toHaveBeenCalled(); + expect(controls().session).not.toBeNull(); + act(() => root.unmount()); + }); + + it("cancels on Escape", () => { + const element = heading("Original"); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + element.textContent = "half-typed"; + press(element, "Escape"); + + expect(element.textContent).toBe("Original"); + expect(onCommit).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it("commits when the element loses focus", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + element.textContent = "Clicked away"; + act(() => element.dispatchEvent(new FocusEvent("blur"))); + + expect(onCommit.mock.calls).toEqual([["Clicked away"]]); + act(() => root.unmount()); + }); + + it("stops listening once the session is over", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + act(() => controls().commit()); + + press(element, "Enter"); + + expect(onCommit).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + }); + }); + + // The mark that says the caret is in the text, not that the element is + // selected. It has to live in the same document as the caret to read so. + it("outlines the element while it is being edited, and puts it back after", () => { + const element = heading(); + element.style.outline = "1px dotted red"; + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + // Serialisation order is the browser's; what matters is that it is the + // accent, solid, and thicker than whatever it replaced. + expect(element.style.outline).toContain("#3CE6AC"); + expect(element.style.outline).toContain("2px"); + + act(() => controls().cancel()); + expect(element.style.outline).toContain("dotted"); + expect(element.style.outline).toContain("red"); + expect(element.style.getPropertyValue("outline-offset")).toBe(""); + act(() => root.unmount()); + }); + + // Opening on a point is what makes this feel like text rather than a dialog. + it("opens the caret where the press landed when it can resolve one", async () => { + const element = heading("Motion Playground"); + const range = document.createRange(); + range.setStart(element.firstChild!, 6); + range.collapse(true); + const doc = document as Document & { caretRangeFromPoint?: unknown }; + const original = doc.caretRangeFromPoint; + doc.caretRangeFromPoint = () => range; + + const { controls, root } = mount(); + act(() => { + controls().start(element, { x: 120, y: 40 }); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + + expect(document.getSelection()?.anchorOffset).toBe(6); + doc.caretRangeFromPoint = original; + act(() => root.unmount()); + }); + + // A point that resolves outside the element (a rotated glyph, a gap) must + // not put the caret in someone else's text. + it("falls back to the end when the point lands outside the element", async () => { + const element = heading("Motion Playground"); + const stranger = heading("Somewhere else"); + const strayRange = document.createRange(); + strayRange.setStart(stranger.firstChild!, 3); + strayRange.collapse(true); + const doc = document as Document & { caretRangeFromPoint?: unknown }; + const original = doc.caretRangeFromPoint; + doc.caretRangeFromPoint = () => strayRange; + + const { controls, root } = mount(); + act(() => { + controls().start(element, { x: 9999, y: 9999 }); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + + const selection = document.getSelection()!; + expect(selection.anchorNode?.parentElement).toBe(element); + expect(selection.anchorOffset).toBe(element.textContent!.length); + doc.caretRangeFromPoint = original; + act(() => root.unmount()); + }); + + // Double click takes the word and triple click takes the lot, in this element + // exactly as in any other text field, because nothing here interferes with + // either. Claiming the double click for select-all cost the word selection. + it("leaves double and triple click to the browser", async () => { + const element = heading("Motion Playground"); + const { controls, root } = mount(); + act(() => { + controls().start(element); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + const before = document.getSelection()?.toString(); + + act(() => element.dispatchEvent(new MouseEvent("dblclick", { bubbles: true }))); + + // No handler ran, so the selection is untouched: the real browser would + // have set it to the word under the pointer before this ever fired. + expect(document.getSelection()?.toString()).toBe(before); + act(() => root.unmount()); + }); +}); + +// Styling a run of characters is what the element is edited in place for, and +// it only counts once the markup survives the trip out of the element. +describe("useInlineTextEdit with styled runs", () => { + it("hands over the markup, not just the words", () => { + const element = heading("hello"); + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + element.innerHTML = 'hello'; + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledWith('hello'); + act(() => root.unmount()); + }); + + it("cleans what it hands over, so the preview shows what will be saved", () => { + const element = heading("hello"); + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + element.innerHTML = 'hi'; + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledWith("hi"); + // Cleaned in the element too, not only on the way out. + expect(element.innerHTML).toBe("hi"); + act(() => root.unmount()); + }); + + it("puts the styling back on cancel, not just the letters", () => { + const element = heading(); + element.innerHTML = 'before'; + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + element.innerHTML = "after"; + act(() => controls().cancel()); + + expect(element.innerHTML).toBe('before'); + expect(onCommit).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it("keeps the element markup-free when nothing was styled", () => { + const element = heading("plain words"); + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledWith("plain words"); + act(() => root.unmount()); + }); + + it("pastes the words, not the page they came from", () => { + const element = heading("hi"); + const { controls, root } = mount(); + act(() => { + controls().start(element); + }); + + const insertText = vi.fn(); + (document as Document & { execCommand: unknown }).execCommand = insertText; + const paste = new Event("paste", { bubbles: true, cancelable: true }) as ClipboardEvent; + Object.defineProperty(paste, "clipboardData", { + value: { getData: (type: string) => (type === "text/plain" ? "pasted" : "pasted") }, + }); + act(() => void element.dispatchEvent(paste)); + + expect(paste.defaultPrevented).toBe(true); + expect(insertText).toHaveBeenCalledWith("insertText", false, "pasted"); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/hooks/useInlineTextEdit.ts b/packages/studio/src/hooks/useInlineTextEdit.ts new file mode 100644 index 0000000000..3282bbbc94 --- /dev/null +++ b/packages/studio/src/hooks/useInlineTextEdit.ts @@ -0,0 +1,265 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { sanitizeRichTextChildren } from "@hyperframes/core/rich-text-sanitize"; + +/** + * Editing an element's text where it sits, in the composition itself. + * + * The alternative is an input positioned over the element, which has to + * reproduce its font, size, weight, spacing, colour, alignment and wrapping to + * look right, and is subtly wrong the moment any of those is missed. The + * preview is a same-origin document holding the real element, and the commit + * path already mutates that exact node, so the element is both the most + * accurate surface to type into and the one the rest of the code understands. + * + * The session owns the element's editable state for its whole life, and tears + * down the same way whichever way it ends. A session that failed to close + * would leave the canvas unable to select anything. + */ + +/** + * `true`, not `plaintext-only`. + * + * `plaintext-only` was what kept a text edit from becoming a structural one, + * and it also made it impossible to give three characters a colour, which is + * the point of editing in the composition rather than in a field. The guard it + * was providing is rebuilt as two narrower ones that do not cost the feature: + * paste arrives as plain text, and what leaves the element goes through the + * sanitiser before anyone writes it to a file. + */ +const EDITABLE = "true"; +/** Studio's accent, so the mark belongs to Studio rather than to the design. */ +const EDITING_OUTLINE = "2px solid #3CE6AC"; + +export interface InlineTextEditSession { + element: HTMLElement; + /** + * The element's markup when editing started, for putting back on cancel. + * Markup rather than text: cancelling an edit that recoloured a word has to + * restore the colours it replaced, not just the letters. + */ + original: string; + /** The element's own outline, to put back when the session ends. */ + outline: string; +} + +export interface InlineTextEditControls { + session: InlineTextEditSession | null; + /** + * Begin editing this element. `caretAt` is a point in the element's own + * document, so the caret can open where the user pointed rather than at a + * fixed end. Returns false when a session is already open. + */ + start: (element: HTMLElement, caretAt?: { x: number; y: number }) => boolean; + /** Hand the current text to the commit function and close. */ + commit: () => void; + /** Put the original text back and close, persisting nothing. */ + cancel: () => void; +} + +export function useInlineTextEdit({ + onCommit, + onPause, +}: { + /** Where the edited text goes. The caller owns persistence. */ + onCommit: (text: string) => void; + /** Stop playback, so the element is not animating under the caret. */ + onPause?: () => void; +}): InlineTextEditControls { + const [session, setSession] = useState(null); + // The teardown reads this rather than the state, so an exit path that runs + // before React re-renders still sees the element it has to clean up. + const openRef = useRef(null); + /** The pending caret placement, so a session that closes first can drop it. */ + const framesRef = useRef(null); + + const teardown = useCallback((): InlineTextEditSession | null => { + const open = openRef.current; + if (!open) return null; + if (framesRef.current !== null) { + open.element.ownerDocument.defaultView?.cancelAnimationFrame(framesRef.current); + framesRef.current = null; + } + openRef.current = null; + setSession(null); + // An element removed from the document mid-session is not an error, it is + // just nothing left to clean up. + if (open.element.isConnected) { + open.element.removeAttribute("contenteditable"); + // Restored rather than cleared: the composition may have authored one. + open.element.style.outline = open.outline; + open.element.style.removeProperty("outline-offset"); + open.element.blur(); + } + return open; + }, []); + + const start = useCallback( + (element: HTMLElement, caretAt?: { x: number; y: number }): boolean => { + if (openRef.current) return false; + + const open = { + element, + original: element.innerHTML, + outline: element.style.outline, + }; + // Drawn on the element itself, not in Studio's overlay above it. This is + // the only mark that says the caret is in the TEXT rather than the + // element being selected, and it has to sit in the same document as the + // caret to read that way. + element.style.outline = EDITING_OUTLINE; + element.style.outlineOffset = "2px"; + openRef.current = open; + setSession(open); + onPause?.(); + + element.setAttribute("contenteditable", EDITABLE); + // Focused and selected on the next frame, not now. The press that opened + // this is still in flight: the canvas overlay takes focus on its own + // pointer-down, and the click that follows puts a caret in the element + // and collapses any selection. Doing it after all of that is what lands. + const view = element.ownerDocument.defaultView; + const raf = view?.requestAnimationFrame(() => { + element.focus({ preventScroll: true }); + placeCaret(element, caretAt); + }); + framesRef.current = raf ?? null; + return true; + }, + [onPause], + ); + + const commit = useCallback(() => { + const open = openRef.current; + if (!open) return; + // Sanitised here, in the element, so the preview shows exactly what will be + // saved rather than something the server will quietly cut down. + sanitizeRichTextChildren(open.element); + const html = open.element.innerHTML; + teardown(); + // After teardown, so the commit path's own resync does not fight an + // element that is still editable. + onCommit(html); + }, [onCommit, teardown]); + + const cancel = useCallback(() => { + const open = openRef.current; + if (!open) return; + if (open.element.isConnected) open.element.innerHTML = open.original; + teardown(); + }, [teardown]); + + // The keys belong to the element, not to the document: the element lives in + // the preview's own document, so a listener on Studio's would never see them. + useEffect(() => { + const element = session?.element; + if (!element) return; + + const onKeyDown = (event: KeyboardEvent) => { + // Shift+Enter is a line break in a multi-line element, and is left alone. + if (event.key === "Enter" && !event.shiftKey) { + event.preventDefault(); + commit(); + return; + } + if (event.key === "Escape") { + event.preventDefault(); + cancel(); + } + }; + // Clicking away keeps the work, which is what every other field in Studio + // does and what a user who has just typed something expects. + const onBlur = () => commit(); + // Nothing here for double or triple click: the browser already takes the + // word on two and the whole text on three, which is what a text field does + // everywhere else. Overriding the double click to take everything cost the + // word selection and gained nothing the triple click did not already do. + // Dropping `plaintext-only` means the browser would otherwise paste a whole + // web page's markup straight in. What arrives is the words. + const onPaste = (event: ClipboardEvent) => { + event.preventDefault(); + const text = event.clipboardData?.getData("text/plain") ?? ""; + if (text) element.ownerDocument.execCommand("insertText", false, text); + }; + + element.addEventListener("keydown", onKeyDown); + element.addEventListener("blur", onBlur); + element.addEventListener("paste", onPaste); + return () => { + element.removeEventListener("keydown", onKeyDown); + element.removeEventListener("blur", onBlur); + element.removeEventListener("paste", onPaste); + }; + }, [session, commit, cancel]); + + return { session, start, commit, cancel }; +} + +/** + * Put the caret where the user pointed, or after the last character. + * + * Opening on a point is what makes this feel like text rather than a dialog: + * the caret lands between the two letters that were clicked, exactly as it + * would in any other editor. + */ +function placeCaret(element: HTMLElement, at?: { x: number; y: number }): void { + const doc = element.ownerDocument; + const selection = doc.defaultView?.getSelection(); + if (!selection) return; + + const range = at ? caretRangeAt(doc, at) : null; + if (range && element.contains(range.startContainer)) { + selection.removeAllRanges(); + selection.addRange(range); + return; + } + placeCaretAtEnd(element); +} + +/** The caret position under a point, across the two APIs browsers expose. */ +function caretRangeAt(doc: Document, at: { x: number; y: number }): Range | null { + const legacy = doc as Document & { + caretRangeFromPoint?: (x: number, y: number) => Range | null; + }; + if (typeof legacy.caretRangeFromPoint === "function") { + return legacy.caretRangeFromPoint(at.x, at.y); + } + const standard = doc as Document & { + caretPositionFromPoint?: (x: number, y: number) => { offsetNode: Node; offset: number } | null; + }; + const position = standard.caretPositionFromPoint?.(at.x, at.y); + if (!position) return null; + const range = doc.createRange(); + range.setStart(position.offsetNode, position.offset); + range.collapse(true); + return range; +} + +/** + * Put the caret after the last character, with nothing selected. + * + * Selecting the whole text would mean the next keystroke silently destroys it, + * which is a bad thing to do to someone who double-clicked to fix a typo. A + * caret at the end is where a person who wants to keep typing expects to be, + * and everything else stays available: click anywhere to move it, drag to + * select, Cmd+A to take the lot. + */ +function placeCaretAtEnd(element: HTMLElement): void { + const doc = element.ownerDocument; + const selection = doc.defaultView?.getSelection(); + if (!selection) return; + const range = doc.createRange(); + // Into the text node, not just past the last child: collapsing the element's + // contents leaves the caret at a node boundary, which types in the right + // place but reports itself as "after child 0" and is a different position + // from the one the user sees at the end of the word. + const last = element.lastChild; + if (last && last.nodeType === 3) { + range.setStart(last, last.textContent?.length ?? 0); + range.collapse(true); + } else { + range.selectNodeContents(element); + range.collapse(false); + } + selection.removeAllRanges(); + selection.addRange(range); +} diff --git a/packages/studio/src/player/lib/playbackShortcuts.ts b/packages/studio/src/player/lib/playbackShortcuts.ts index e30e7192be..da312899d6 100644 --- a/packages/studio/src/player/lib/playbackShortcuts.ts +++ b/packages/studio/src/player/lib/playbackShortcuts.ts @@ -6,15 +6,13 @@ * is active and the user is navigating caption segments). */ +import { isTypingTarget } from "../../utils/typingTarget"; + const PLAYBACK_FRAME_STEP_CODES = new Set(["ArrowLeft", "ArrowRight"]); const PLAYBACK_SHORTCUT_IGNORED_SELECTOR = [ - "input", - "textarea", - "select", "button", "a[href]", - "[contenteditable='true']", "[role='button']", "[role='checkbox']", "[role='combobox']", @@ -27,6 +25,9 @@ const PLAYBACK_SHORTCUT_IGNORED_SELECTOR = [ ].join(","); export function shouldIgnorePlaybackShortcutTarget(target: EventTarget | null): boolean { + // Anything the user is typing into owns its keys outright, editable elements + // included: a letter claimed here never reaches the text. + if (isTypingTarget(target)) return true; if (!target || typeof target !== "object") return false; const candidate = target as { closest?: unknown }; if (typeof candidate.closest !== "function") return false; diff --git a/packages/studio/src/utils/studioHelpers.ts b/packages/studio/src/utils/studioHelpers.ts index 5cef1add34..489bf8ea0b 100644 --- a/packages/studio/src/utils/studioHelpers.ts +++ b/packages/studio/src/utils/studioHelpers.ts @@ -1,3 +1,4 @@ +import { isTypingTarget } from "./typingTarget"; import type { TimelineElement } from "../player/store/playerStore"; import type { DomEditSelection } from "../components/editor/domEditing"; import type { TimelineAssetKind } from "./timelineAssetDrop"; @@ -114,11 +115,7 @@ export function getEventTargetElement(target: EventTarget | null): HTMLElement | } export function shouldIgnoreHistoryShortcut(target: EventTarget | null): boolean { - const el = getEventTargetElement(target); - if (!el) return false; - return Boolean( - el.closest("input, textarea, select, [contenteditable='true'], [role='textbox'], .cm-editor"), - ); + return isTypingTarget(target); } export function getHistoryShortcutLabel(action: "undo" | "redo"): string { diff --git a/packages/studio/src/utils/typingTarget.test.ts b/packages/studio/src/utils/typingTarget.test.ts new file mode 100644 index 0000000000..5f5e1211f8 --- /dev/null +++ b/packages/studio/src/utils/typingTarget.test.ts @@ -0,0 +1,47 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it } from "vitest"; +import { isTypingTarget } from "./typingTarget"; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function mount(html: string): HTMLElement { + document.body.innerHTML = html; + return document.body.firstElementChild as HTMLElement; +} + +describe("isTypingTarget", () => { + // The bug this exists for: inline text editing uses plaintext-only, an + // attribute selector for 'true' missed it, and the playback shortcuts ate + // every letter typed into the composition. + it("recognises a plaintext-only editable, not just contenteditable=true", () => { + expect(isTypingTarget(mount('

Hi

'))).toBe(true); + expect(isTypingTarget(mount('

Hi

'))).toBe(true); + expect(isTypingTarget(mount("

Hi

"))).toBe(true); + }); + + it("recognises a child of an editable, which a selector on the target alone misses", () => { + const host = mount('
inner
'); + expect(isTypingTarget(host.querySelector("span"))).toBe(true); + }); + + it("recognises the ordinary fields too", () => { + expect(isTypingTarget(mount(""))).toBe(true); + expect(isTypingTarget(mount(""))).toBe(true); + expect(isTypingTarget(mount(""))).toBe(true); + expect(isTypingTarget(mount('
'))).toBe(true); + }); + + it("leaves the keys alone for anything that is not being typed into", () => { + expect(isTypingTarget(mount("
plain
"))).toBe(false); + expect(isTypingTarget(mount(""))).toBe(false); + expect(isTypingTarget(mount('

Hi

'))).toBe(false); + }); + + it("says no to nothing at all", () => { + expect(isTypingTarget(null)).toBe(false); + expect(isTypingTarget({} as EventTarget)).toBe(false); + }); +}); diff --git a/packages/studio/src/utils/typingTarget.ts b/packages/studio/src/utils/typingTarget.ts new file mode 100644 index 0000000000..eb91626c62 --- /dev/null +++ b/packages/studio/src/utils/typingTarget.ts @@ -0,0 +1,39 @@ +/** + * Whether a keystroke is going somewhere the user is typing. + * + * Every keyboard shortcut in Studio has to ask this before claiming a key, and + * they were each asking it slightly differently. The version that matched + * `[contenteditable='true']` missed `contenteditable="plaintext-only"`, which + * is what inline text editing uses, so the playback shortcuts kept claiming + * letters out of it: `a` seeked to the in-point and `e` to the out-point, + * `preventDefault` and all, and the character never reached the text. + * + * `isContentEditable` is the property to ask, not the attribute to match: it is + * true for every editable value and for an element made editable by an + * ancestor, which an attribute selector on the target alone cannot see. + */ +export function isTypingTarget(target: EventTarget | null): boolean { + const element = asElement(target); + if (!element) return false; + if (element.isContentEditable) return true; + return element.closest(TYPING_SELECTOR) !== null; +} + +/** + * Things a keystroke belongs to rather than to a shortcut. `contenteditable` is + * matched by value as well, for a host whose own property is not yet true. + */ +const TYPING_SELECTOR = [ + "input", + "textarea", + "select", + "[contenteditable]:not([contenteditable='false'])", + "[role='textbox']", + ".cm-editor", +].join(","); + +function asElement(target: EventTarget | null): HTMLElement | null { + if (!target || typeof target !== "object") return null; + const candidate = target as { closest?: unknown; isContentEditable?: unknown }; + return typeof candidate.closest === "function" ? (target as HTMLElement) : null; +} From 27e3d852ff55496b5cf3b883b379c6c37b7eaca9 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 11:23:13 -0400 Subject: [PATCH 2/2] refactor(studio): keep domEditingLayers under the size cap The rich-text operation pushed this file past the 600-line gate. Same change the branch made later, landed with the commit that caused it. --- .../src/components/editor/domEditingLayers.ts | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/packages/studio/src/components/editor/domEditingLayers.ts b/packages/studio/src/components/editor/domEditingLayers.ts index 53bd536c65..178d72f50a 100644 --- a/packages/studio/src/components/editor/domEditingLayers.ts +++ b/packages/studio/src/components/editor/domEditingLayers.ts @@ -519,28 +519,13 @@ export function buildDomEditTextPatchOperation( value: string, childLocator?: DomEditChildLocator, ): PatchOperation { - return { - type: "text-content", - property: "text", - value, - ...childLocator, - }; + return { type: "text-content", property: "text", value, ...childLocator }; } -/** - * Replace an element's whole contents with markup, for a change no per-child - * operation can express — a text layer added, removed or reordered. - * - * The same operation an inline style edit uses, because it is the same thing: - * this element's children are now these. Both ends run the markup through the - * one sanitizer before it reaches a file. - */ +/** Replace an element's contents with markup, for a change no per-child operation + * can express (a text layer added, removed or reordered). Sanitized at both ends. */ export function buildDomEditRichTextPatchOperation(value: string): PatchOperation { - return { - type: "rich-text", - property: "", - value, - }; + return { type: "rich-text", property: "", value }; } // ─── Non-editable reason ─────────────────────────────────────────────────────