From 444639d75197c78d43ca1a52fe9f67ca5b5056b9 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 9 Jul 2026 00:19:55 -0700 Subject: [PATCH] fix(studio): gate flat Text group to text-editable elements, dedupe heading in multi-field fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flat inspector's Text FlatGroup rendered unconditionally, showing an empty "Text" header for non-text elements (image, video, etc). Gate it on isTextEditableSelection(element) so it disappears entirely when there's no text to edit. Also, the legacy multi-field TextSection (used as a fallback when an element has 2+ text fields) rendered its own internal "Text" heading nested inside the new flat Text FlatGroup, producing a doubled "Text" heading. Add a hideOwnHeading prop to TextSection (default false, so its other — legacy, non-flat — call site is unaffected) and pass it from FlatTextSection's fallback path. --- .../components/editor/PropertyPanel.test.tsx | 73 ++++++++- .../components/editor/PropertyPanelFlat.tsx | 58 ++++--- .../editor/propertyPanelFlatTextSection.tsx | 4 + .../editor/propertyPanelSections.test.tsx | 26 +++ .../editor/propertyPanelSections.tsx | 152 ++++++++++-------- 5 files changed, 217 insertions(+), 96 deletions(-) diff --git a/packages/studio/src/components/editor/PropertyPanel.test.tsx b/packages/studio/src/components/editor/PropertyPanel.test.tsx index 6b2ac355e..5c3845e59 100644 --- a/packages/studio/src/components/editor/PropertyPanel.test.tsx +++ b/packages/studio/src/components/editor/PropertyPanel.test.tsx @@ -63,7 +63,47 @@ function baseElement() { }; } -async function renderPanel(flatEnabled: boolean) { +// Bug 1 fixture: no text fields at all, so isTextEditableSelection(element) is +// false — the Text FlatGroup must not render (not even empty/collapsed). +function nonTextElement() { + return { + ...baseElement(), + id: "image-clip", + selector: "#image-clip", + label: "Image Clip", + tagName: "img", + textContent: "", + textFields: [], + }; +} + +// 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). +function multiFieldTextElement() { + const base = baseElement(); + return { + ...base, + textFields: [ + base.textFields[0], + { + key: "field-1", + label: "Text", + value: "SECOND FIELD", + tagName: "div", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }, + ], + }; +} + +async function renderPanel( + flatEnabled: boolean, + elementOverride: ReturnType = baseElement(), +) { vi.resetModules(); vi.doMock("./manualEditingAvailability", async () => { const actual = await vi.importActual( @@ -79,9 +119,11 @@ async function renderPanel(flatEnabled: boolean) { // mount (handlers fire on interaction), so cast a minimal object to the full // props shape rather than stubbing all ~15 required fields. const props = { - element: baseElement(), + element: elementOverride, + assets: [], onSetStyle: vi.fn(), onSetText: vi.fn(), + onSetAttributeLive: vi.fn(), } as unknown as PropertyPanelProps; act(() => { root.render(); @@ -139,4 +181,31 @@ describe("PropertyPanel — STUDIO_FLAT_INSPECTOR_ENABLED on", () => { }, RENDER_TIMEOUT_MS, ); + + it( + "renders no Text group at all for a non-text element (bug 1)", + async () => { + const { host, root } = await renderPanel(true, nonTextElement()); + expect(host.querySelector('[data-flat-group-open="true"]')).toBeNull(); + expect(host.querySelector('[data-flat-group-collapsed="true"]')).toBeNull(); + act(() => root.unmount()); + }, + RENDER_TIMEOUT_MS, + ); + + it( + "renders exactly one Text heading for a multi-field text element (bug 2)", + async () => { + 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. + 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. + expect(host.textContent).toContain("Text layers"); + act(() => root.unmount()); + }, + RENDER_TIMEOUT_MS, + ); }); diff --git a/packages/studio/src/components/editor/PropertyPanelFlat.tsx b/packages/studio/src/components/editor/PropertyPanelFlat.tsx index e20b2c729..a6dfca718 100644 --- a/packages/studio/src/components/editor/PropertyPanelFlat.tsx +++ b/packages/studio/src/components/editor/PropertyPanelFlat.tsx @@ -1,6 +1,7 @@ import { useState } from "react"; import { resolveEditingSections } from "@hyperframes/core/editing"; import type { DomEditSelection } from "./domEditing"; +import { isTextEditableSelection } from "./domEditing"; import type { PropertyPanelProps } from "./propertyPanelHelpers"; import { PropertyPanelFlatHeader } from "./PropertyPanelFlatHeader"; import { PropertyPanelFlatFooter } from "./PropertyPanelFlatFooter"; @@ -101,9 +102,14 @@ export function PropertyPanelFlat({ clipboardCopied: boolean; onCopyElementInfo: () => void; }) { + // Defaulting to "text" is harmless for a non-text element even though the + // Text FlatGroup won't render (nothing else reads openGroupId yet) — this + // only matters once a second FlatGroup exists (Plan 2+), at which point a + // non-text element should default-open that group instead. const [openGroupId, setOpenGroupId] = useState("text"); const [pinnedGroupIds, setPinnedGroupIds] = useState([]); + const isTextEditable = isTextEditableSelection(element); const elementKind = sections.media ? "media" : element.textFields.length > 0 ? "text" : "other"; return ( @@ -125,31 +131,33 @@ export function PropertyPanelFlat({ showUngroup={Boolean(onUngroup && element.dataAttributes["hf-group"] != null)} />
- setOpenGroupId((current) => (current === "text" ? "" : "text"))} - onTogglePin={() => - setPinnedGroupIds((current) => - current.includes("text") - ? current.filter((id) => id !== "text") - : [...current, "text"], - ) - } - summary={formatTextFieldPreview(element.textFields[0]?.value ?? "")} - > - - + {isTextEditable && ( + setOpenGroupId((current) => (current === "text" ? "" : "text"))} + onTogglePin={() => + setPinnedGroupIds((current) => + current.includes("text") + ? current.filter((id) => id !== "text") + : [...current, "text"], + ) + } + summary={formatTextFieldPreview(element.textFields[0]?.value ?? "")} + > + + + )} {sections.timing && ( 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 ( ); } diff --git a/packages/studio/src/components/editor/propertyPanelSections.test.tsx b/packages/studio/src/components/editor/propertyPanelSections.test.tsx index 52fc8dd07..fbcff5e0f 100644 --- a/packages/studio/src/components/editor/propertyPanelSections.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelSections.test.tsx @@ -132,4 +132,30 @@ describe("FlatTextSection", () => { expect(onSetTextFieldStyle).toHaveBeenCalledWith("field-0", "font-weight", "700"); act(() => root.unmount()); }); + + it("suppresses TextSection's own heading when falling back for a multi-field element", () => { + const { host, root } = renderSection({ + textFields: [ + makeElement().textFields[0], + { + key: "field-1", + label: "Text", + value: "SECOND FIELD", + tagName: "div", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }, + ], + }); + // 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. + expect(host.querySelector('[data-panel-section="text"]')).toBeNull(); + // The multi-field fallback's own content must still render. + expect(host.textContent).toContain("Text layers"); + act(() => root.unmount()); + }); }); diff --git a/packages/studio/src/components/editor/propertyPanelSections.tsx b/packages/studio/src/components/editor/propertyPanelSections.tsx index 303930526..c7625e230 100644 --- a/packages/studio/src/components/editor/propertyPanelSections.tsx +++ b/packages/studio/src/components/editor/propertyPanelSections.tsx @@ -361,6 +361,7 @@ export function TextSection({ onSetTextFieldStyle, onAddTextField, onRemoveTextField, + hideOwnHeading = false, }: { element: DomEditSelection; styles: Record; @@ -370,6 +371,11 @@ export function TextSection({ onSetTextFieldStyle: (fieldKey: string, property: string, value: string) => void; onAddTextField: (afterFieldKey?: string) => string | Promise | null; onRemoveTextField: (fieldKey: string) => void; + /** Skip TextSection's own "Text" Section heading/wrapper — for callers (the + * flat inspector's multi-field fallback) that already render their own + * "Text" heading one level up, to avoid a doubled heading. Defaults to + * false so the legacy (non-flat) call site is unaffected. */ + hideOwnHeading?: boolean; }) { const hasTextControls = isTextEditableSelection(element); const [activeTextFieldKey, setActiveTextFieldKey] = useState( @@ -391,85 +397,93 @@ export function TextSection({ if (!activeField) return null; if (textFields.length === 1) { + const content = ( + + ); + if (hideOwnHeading) return content; return (
} defaultCollapsed> - + {content}
); } - return ( -
}> -
-
-
- Text layers - -
-
- {textFields.map((field, index) => { - const active = field.key === activeField.key; - return ( - +
+
+ {textFields.map((field, index) => { + const active = field.key === activeField.key; + return ( + - ); - })} -
+ + {field.tagName} + +
+ + ); + })}
-
+ + + ); + if (hideOwnHeading) return content; + return ( +
}> + {content}
); }