fix(studio): gate flat Text group to text-editable elements, dedupe heading in multi-field fallback

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.
2026-09-03 04:38:33 +00:00 · 2026-07-14 00:59:06 -07:00
parent e0066834b7
commit 7b5d8c7d44
5 changed files with 217 additions and 96 deletions
@@ -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 <TextSection> 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<typeof baseElement> = baseElement(),
+) {
  vi.resetModules();
  vi.doMock("./manualEditingAvailability", async () => {
    const actual = await vi.importActual<typeof import("./manualEditingAvailability")>(
@@ -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(<PropertyPanel {...props} />);
@@ -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,
+  );
 });
@@ -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<string>("text");
  const [pinnedGroupIds, setPinnedGroupIds] = useState<string[]>([]);

+  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)}
      />
      <div className="flex-1 overflow-y-auto">
-        <FlatGroup
-          title="Text"
-          isOpen={openGroupId === "text" || pinnedGroupIds.includes("text")}
-          isPinned={pinnedGroupIds.includes("text")}
-          onToggleOpen={() => setOpenGroupId((current) => (current === "text" ? "" : "text"))}
-          onTogglePin={() =>
-            setPinnedGroupIds((current) =>
-              current.includes("text")
-                ? current.filter((id) => id !== "text")
-                : [...current, "text"],
-            )
-          }
-          summary={formatTextFieldPreview(element.textFields[0]?.value ?? "")}
-        >
-          <FlatTextSection
-            element={element}
-            styles={styles}
-            fontAssets={fontAssets}
-            onImportFonts={onImportFonts}
-            onSetText={onSetText}
-            onSetTextFieldStyle={onSetTextFieldStyle}
-            onAddTextField={onAddTextField}
-            onRemoveTextField={onRemoveTextField}
-          />
-        </FlatGroup>
+        {isTextEditable && (
+          <FlatGroup
+            title="Text"
+            isOpen={openGroupId === "text" || pinnedGroupIds.includes("text")}
+            isPinned={pinnedGroupIds.includes("text")}
+            onToggleOpen={() => setOpenGroupId((current) => (current === "text" ? "" : "text"))}
+            onTogglePin={() =>
+              setPinnedGroupIds((current) =>
+                current.includes("text")
+                  ? current.filter((id) => id !== "text")
+                  : [...current, "text"],
+              )
+            }
+            summary={formatTextFieldPreview(element.textFields[0]?.value ?? "")}
+          >
+            <FlatTextSection
+              element={element}
+              styles={styles}
+              fontAssets={fontAssets}
+              onImportFonts={onImportFonts}
+              onSetText={onSetText}
+              onSetTextFieldStyle={onSetTextFieldStyle}
+              onAddTextField={onAddTextField}
+              onRemoveTextField={onRemoveTextField}
+            />
+          </FlatGroup>
+        )}

        {sections.timing && (
          <TimingSection
@@ -206,6 +206,9 @@ export function FlatTextSection({
  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 (
      <TextSection
        element={element}
@@ -216,6 +219,7 @@ export function FlatTextSection({
        onSetTextFieldStyle={onSetTextFieldStyle}
        onAddTextField={onAddTextField}
        onRemoveTextField={onRemoveTextField}
+        hideOwnHeading
      />
    );
  }
@@ -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());
+  });
 });
@@ -382,6 +382,7 @@ export function TextSection({
  onSetTextFieldStyle,
  onAddTextField,
  onRemoveTextField,
+  hideOwnHeading = false,
 }: {
  element: DomEditSelection;
  styles: Record<string, string>;
@@ -391,6 +392,11 @@ export function TextSection({
  onSetTextFieldStyle: (fieldKey: string, property: string, value: string) => void;
  onAddTextField: (afterFieldKey?: string) => string | Promise<string | null> | 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<string | null>(
@@ -412,85 +418,93 @@ export function TextSection({
  if (!activeField) return null;

  if (textFields.length === 1) {
+    const content = (
+      <TextFieldEditor
+        field={activeField}
+        styles={styles}
+        fontAssets={fontAssets}
+        onImportFonts={onImportFonts}
+        showRemove={false}
+        onSetText={onSetText}
+        onSetTextFieldStyle={onSetTextFieldStyle}
+        onRemoveTextField={onRemoveTextField}
+      />
+    );
+    if (hideOwnHeading) return content;
    return (
      <Section title="Text" icon={<Type size={15} />} defaultCollapsed>
-        <TextFieldEditor
-          field={activeField}
-          styles={styles}
-          fontAssets={fontAssets}
-          onImportFonts={onImportFonts}
-          showRemove={false}
-          onSetText={onSetText}
-          onSetTextFieldStyle={onSetTextFieldStyle}
-          onRemoveTextField={onRemoveTextField}
-        />
+        {content}
      </Section>
    );
  }

-  return (
-    <Section title="Text" icon={<Type size={15} />}>
-      <div className="space-y-4">
-        <div className="grid gap-1.5">
-          <div className="flex min-w-0 flex-wrap items-center justify-between gap-2">
-            <span className={LABEL}>Text layers</span>
-            <button
-              type="button"
-              onClick={() => {
-                void Promise.resolve(onAddTextField(activeField.key)).then((nextKey) => {
-                  if (nextKey) setActiveTextFieldKey(nextKey);
-                });
-              }}
-              className="inline-flex h-7 max-w-full items-center gap-1.5 rounded-lg border border-neutral-700 bg-neutral-950 px-2.5 text-[11px] font-medium text-neutral-300 transition-colors hover:border-neutral-600 hover:text-white"
-            >
-              <Plus size={12} className="flex-shrink-0" />
-              <span className="truncate">Add text</span>
-            </button>
-          </div>
-          <div className="grid gap-2">
-            {textFields.map((field, index) => {
-              const active = field.key === activeField.key;
-              return (
-                <button
-                  key={field.key}
-                  type="button"
-                  onClick={() => setActiveTextFieldKey(field.key)}
-                  className={`min-w-0 w-full rounded-xl border px-3 py-2 text-left transition-colors ${
-                    active
-                      ? "border-studio-accent/50 bg-studio-accent/10"
-                      : "border-neutral-800 bg-neutral-900/80 hover:border-neutral-700 hover:bg-neutral-900"
-                  }`}
-                >
-                  <div className="flex min-w-0 items-center justify-between gap-2">
-                    <div className="flex min-w-0 items-center gap-2">
-                      <span
-                        className="h-4 w-4 flex-shrink-0 rounded border border-neutral-700 bg-neutral-950"
-                        style={{ backgroundColor: getTextFieldColor(field, styles) }}
-                      />
-                      <span className="min-w-0 truncate text-[11px] font-medium text-neutral-100">
-                        {formatTextFieldPreview(field.value) || `Text ${index + 1}`}
-                      </span>
-                    </div>
-                    <span className="flex-shrink-0 rounded-md border border-neutral-700 bg-neutral-950 px-1.5 py-0.5 text-[10px] text-neutral-500">
-                      {field.tagName}
+  const content = (
+    <div className="space-y-4">
+      <div className="grid gap-1.5">
+        <div className="flex min-w-0 flex-wrap items-center justify-between gap-2">
+          <span className={LABEL}>Text layers</span>
+          <button
+            type="button"
+            onClick={() => {
+              void Promise.resolve(onAddTextField(activeField.key)).then((nextKey) => {
+                if (nextKey) setActiveTextFieldKey(nextKey);
+              });
+            }}
+            className="inline-flex h-7 max-w-full items-center gap-1.5 rounded-lg border border-neutral-700 bg-neutral-950 px-2.5 text-[11px] font-medium text-neutral-300 transition-colors hover:border-neutral-600 hover:text-white"
+          >
+            <Plus size={12} className="flex-shrink-0" />
+            <span className="truncate">Add text</span>
+          </button>
+        </div>
+        <div className="grid gap-2">
+          {textFields.map((field, index) => {
+            const active = field.key === activeField.key;
+            return (
+              <button
+                key={field.key}
+                type="button"
+                onClick={() => setActiveTextFieldKey(field.key)}
+                className={`min-w-0 w-full rounded-xl border px-3 py-2 text-left transition-colors ${
+                  active
+                    ? "border-studio-accent/50 bg-studio-accent/10"
+                    : "border-neutral-800 bg-neutral-900/80 hover:border-neutral-700 hover:bg-neutral-900"
+                }`}
+              >
+                <div className="flex min-w-0 items-center justify-between gap-2">
+                  <div className="flex min-w-0 items-center gap-2">
+                    <span
+                      className="h-4 w-4 flex-shrink-0 rounded border border-neutral-700 bg-neutral-950"
+                      style={{ backgroundColor: getTextFieldColor(field, styles) }}
+                    />
+                    <span className="min-w-0 truncate text-[11px] font-medium text-neutral-100">
+                      {formatTextFieldPreview(field.value) || `Text ${index + 1}`}
                    </span>
                  </div>
-                </button>
-              );
-            })}
-          </div>
+                  <span className="flex-shrink-0 rounded-md border border-neutral-700 bg-neutral-950 px-1.5 py-0.5 text-[10px] text-neutral-500">
+                    {field.tagName}
+                  </span>
+                </div>
+              </button>
+            );
+          })}
        </div>
-        <TextFieldEditor
-          field={activeField}
-          styles={styles}
-          fontAssets={fontAssets}
-          onImportFonts={onImportFonts}
-          showRemove={true}
-          onSetText={onSetText}
-          onSetTextFieldStyle={onSetTextFieldStyle}
-          onRemoveTextField={onRemoveTextField}
-        />
      </div>
+      <TextFieldEditor
+        field={activeField}
+        styles={styles}
+        fontAssets={fontAssets}
+        onImportFonts={onImportFonts}
+        showRemove={true}
+        onSetText={onSetText}
+        onSetTextFieldStyle={onSetTextFieldStyle}
+        onRemoveTextField={onRemoveTextField}
+      />
+    </div>
+  );
+  if (hideOwnHeading) return content;
+  return (
+    <Section title="Text" icon={<Type size={15} />}>
+      {content}
    </Section>
  );
 }