From 0524f1c9c2a4a9fe1c4e2c6f6d81be6e9f6d5f5c Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 9 Jul 2026 18:21:29 -0700 Subject: [PATCH] feat(studio): flatten multi-field Text, retiring TextSection fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FlatTextSection's multi-field branch (textFields.length > 1) now renders FlatTextLayerList (Task 5) + the existing single-field FlatTextFieldEditor for the active field, tracked via new local activeFieldKey state that resyncs (useEffect) when the active field disappears from props. This retires the legacy TextSection delegation entirely for that case; the TextSection import is removed from propertyPanelFlatTextSection.tsx since nothing else in the file referenced it. Also updates propertyPanelSections.test.tsx and PropertyPanel.test.tsx, which exercised/documented the old multi-field-falls-back-to-legacy- TextSection behavior in comments and test titles — reworded to describe the new flat path (assertions were already compatible and still pass). Flag for reviewer: hideOwnHeading on the legacy TextSection component (propertyPanelSections.tsx) was added in an earlier plan specifically for this now-removed call site. It has no remaining consumer after this task lands (PropertyPanel.tsx's legacy caller doesn't pass it). Left in place per brief instruction — not deleting unilaterally, since that's a scope decision for whoever reviews this task. Co-Authored-By: Claude Sonnet 5 --- .../components/editor/PropertyPanel.test.tsx | 12 +- .../propertyPanelFlatTextSection.test.tsx | 168 +++++++++++++++++- .../editor/propertyPanelFlatTextSection.tsx | 52 ++++-- .../editor/propertyPanelSections.test.tsx | 12 +- 4 files changed, 215 insertions(+), 29 deletions(-) diff --git a/packages/studio/src/components/editor/PropertyPanel.test.tsx b/packages/studio/src/components/editor/PropertyPanel.test.tsx index 27007860b..d30376854 100644 --- a/packages/studio/src/components/editor/PropertyPanel.test.tsx +++ b/packages/studio/src/components/editor/PropertyPanel.test.tsx @@ -81,9 +81,10 @@ function nonTextElement() { }; } -// Bug 2 fixture: 2+ text fields, which routes FlatTextSection to the legacy -// multi-field fallback — must not double-render the "Text" -// heading (FlatGroup's own heading + TextSection's internal Section heading). +// Bug 2 fixture: 2+ text fields, which routes FlatTextSection to its own +// flat multi-field layer list (FlatTextLayerList + FlatTextFieldEditor) — +// must not double-render the "Text" heading (FlatGroup's own heading; this +// component never renders one of its own). function multiFieldTextElement() { const base = baseElement(); return { @@ -312,10 +313,11 @@ describe("PropertyPanel — STUDIO_FLAT_INSPECTOR_ENABLED on", () => { const { host, root } = await renderPanel(true, multiFieldTextElement()); // The FlatGroup's own "Text" heading is the only one that should exist — // the legacy TextSection's internal Section heading (data-panel-section - // ="text") must be suppressed when it's used as the flat fallback. + // ="text") must never appear, since the flat multi-field path no longer + // delegates to that component at all. expect(host.querySelector('[data-flat-group-open="true"]')).not.toBeNull(); expect(host.querySelector('[data-panel-section="text"]')).toBeNull(); - // Content from the legacy multi-field fallback must still render. + // Content from the flat multi-field layer list must render. expect(host.textContent).toContain("Text layers"); act(() => root.unmount()); }, diff --git a/packages/studio/src/components/editor/propertyPanelFlatTextSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatTextSection.test.tsx index be526bb2a..4ca2cb36d 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatTextSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatTextSection.test.tsx @@ -1,9 +1,10 @@ // @vitest-environment happy-dom -import React, { act } from "react"; +import React, { act, useState } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { FlatTextLayerList } from "./propertyPanelFlatTextSection"; +import { FlatTextLayerList, FlatTextSection } from "./propertyPanelFlatTextSection"; +import type { DomEditSelection, DomEditTextField } from "./domEditingTypes"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -86,3 +87,166 @@ describe("FlatTextLayerList", () => { act(() => root.unmount()); }); }); + +function makeMultiFieldElement(): DomEditSelection { + return { + element: document.createElement("div"), + id: "multi", + selector: ".multi", + label: "Multi", + tagName: "div", + sourceFile: "index.html", + compositionPath: "index.html", + isCompositionHost: false, + isInsideLockedComposition: false, + boundingBox: { x: 0, y: 0, width: 100, height: 100 }, + textContent: "Headline Subhead", + dataAttributes: {}, + inlineStyles: {}, + computedStyles: {}, + textFields: [ + { + key: "a", + label: "Text", + value: "Headline", + tagName: "div", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }, + { + key: "b", + label: "Text", + value: "Subhead", + tagName: "span", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }, + ], + capabilities: { + canSelect: true, + canEditStyles: true, + canCrop: true, + canMove: true, + canResize: true, + canApplyManualOffset: true, + canApplyManualSize: true, + canApplyManualRotation: true, + }, + } as DomEditSelection; +} + +describe("FlatTextSection — multi-field", () => { + it("shows the layer list, switches the active field's rows on selection, and has no doubled heading (this component never renders its own heading — the parent FlatGroup does)", () => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + expect(host.textContent).toContain("Headline"); + expect(host.textContent).toContain("Subhead"); + // Active field's editor rows are visible (Font/Weight/etc. from FlatTextFieldEditor). + expect(host.textContent).toContain("Weight"); + // Exactly one "Text layers" micro-label — this component doesn't duplicate its own list. + const layerLabels = Array.from(host.querySelectorAll("div")).filter( + (el) => el.textContent === "Text layers", + ); + expect(layerLabels.length).toBeLessThanOrEqual(1); + + const rows = host.querySelectorAll('[data-flat-text-layer-row="true"]'); + act(() => rows[1].dispatchEvent(new MouseEvent("click", { bubbles: true }))); + expect(host.textContent).toContain("Subhead"); + act(() => root.unmount()); + }); + + it("wires onAdd/onRemove end-to-end: async onAddTextField switches the active field once it appears in props, and the resync effect falls back to the first field when the active one disappears", async () => { + let addResolved = false; + + function Harness() { + const [fields, setFields] = useState(makeMultiFieldElement().textFields); + const element: DomEditSelection = { ...makeMultiFieldElement(), textFields: fields }; + return ( + + Promise.resolve().then(() => { + addResolved = true; + setFields((prev) => [ + ...prev, + { + key: "c", + label: "Text", + value: "Third", + tagName: "div", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }, + ]); + return "c"; + }) + } + onRemoveTextField={(fieldKey: string) => + setFields((prev) => prev.filter((field) => field.key !== fieldKey)) + } + /> + ); + } + + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render(); + }); + + let rows = host.querySelectorAll('[data-flat-text-layer-row="true"]'); + expect(rows).toHaveLength(2); + expect((rows[0] as HTMLElement).getAttribute("data-active")).toBe("true"); + + const addButton = host.querySelector('[data-flat-text-layer-add="true"]'); + await act(async () => { + addButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(addResolved).toBe(true); + rows = host.querySelectorAll('[data-flat-text-layer-row="true"]'); + expect(rows).toHaveLength(3); + expect((rows[2] as HTMLElement).getAttribute("data-active")).toBe("true"); + + // Remove the active field ("c") through the wired onRemoveTextField — the + // resync useEffect must fall back to the first remaining field ("a") + // since "c" no longer exists in element.textFields. + const removeButtons = host.querySelectorAll( + '[data-flat-text-layer-remove="true"]', + ); + act(() => { + removeButtons[2].dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + rows = host.querySelectorAll('[data-flat-text-layer-row="true"]'); + expect(rows).toHaveLength(2); + expect((rows[0] as HTMLElement).getAttribute("data-active")).toBe("true"); + + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/components/editor/propertyPanelFlatTextSection.tsx b/packages/studio/src/components/editor/propertyPanelFlatTextSection.tsx index fd0c14090..455053891 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatTextSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatTextSection.tsx @@ -1,3 +1,4 @@ +import { useEffect, useState } from "react"; import { Plus, X } from "../../icons/SystemIcons"; import { isTextEditableSelection, type DomEditSelection } from "./domEditing"; import type { ImportedFontAsset } from "./fontAssets"; @@ -16,7 +17,6 @@ import { getTextFieldColor, getTextStyleValue, TextAreaField, - TextSection, WEIGHT_LABELS, } from "./propertyPanelSections"; @@ -201,27 +201,47 @@ export function FlatTextSection({ onAddTextField: (afterFieldKey?: string) => string | Promise | null; onRemoveTextField: (fieldKey: string) => void; }) { + const [activeFieldKey, setActiveFieldKey] = useState( + element.textFields[0]?.key ?? null, + ); + + useEffect(() => { + const nextFields = element.textFields; + setActiveFieldKey((current) => { + if (current && nextFields.some((field) => field.key === current)) return current; + return nextFields[0]?.key ?? null; + }); + }, [element.id, element.selector, element.textFields]); + if (!isTextEditableSelection(element)) return null; const textFields = element.textFields; - const activeField = textFields[0]; + const activeField = textFields.find((field) => field.key === activeFieldKey) ?? textFields[0]; if (!activeField) return null; if (textFields.length > 1) { - // The parent FlatGroup (PropertyPanelFlat) already renders a "Text" - // heading around this section — suppress TextSection's own internal - // heading so the flat panel doesn't show "Text" twice in a row. return ( - +
+ + void Promise.resolve(onAddTextField(activeField.key)).then((nextKey) => { + if (nextKey) setActiveFieldKey(nextKey); + }) + } + onRemove={onRemoveTextField} + /> + +
); } diff --git a/packages/studio/src/components/editor/propertyPanelSections.test.tsx b/packages/studio/src/components/editor/propertyPanelSections.test.tsx index fbcff5e0f..075915ed1 100644 --- a/packages/studio/src/components/editor/propertyPanelSections.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelSections.test.tsx @@ -133,7 +133,7 @@ describe("FlatTextSection", () => { act(() => root.unmount()); }); - it("suppresses TextSection's own heading when falling back for a multi-field element", () => { + it("renders the flat layer list (not the legacy TextSection) for a multi-field element", () => { const { host, root } = renderSection({ textFields: [ makeElement().textFields[0], @@ -149,12 +149,12 @@ describe("FlatTextSection", () => { }, ], }); - // TextSection's own Section wrapper (data-panel-section="text") must not - // render here — the caller (PropertyPanelFlat's FlatGroup) already shows - // a "Text" heading, so a second one from the legacy component would be a - // doubled heading. + // Legacy TextSection's own Section wrapper (data-panel-section="text") + // must never render here — multi-field elements now go through the flat + // FlatTextLayerList + FlatTextFieldEditor path end to end, not a + // delegation to the legacy component. expect(host.querySelector('[data-panel-section="text"]')).toBeNull(); - // The multi-field fallback's own content must still render. + // The flat layer list's own content must render. expect(host.textContent).toContain("Text layers"); act(() => root.unmount()); });