mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
feat(studio): flatten multi-field Text, retiring TextSection fallback
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
475f764e5a
commit
0524f1c9c2
@@ -81,9 +81,10 @@ function nonTextElement() {
|
||||
};
|
||||
}
|
||||
|
||||
// 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).
|
||||
// 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());
|
||||
},
|
||||
|
||||
@@ -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(
|
||||
<FlatTextSection
|
||||
element={makeMultiFieldElement()}
|
||||
styles={{}}
|
||||
fontAssets={[]}
|
||||
onSetText={vi.fn()}
|
||||
onSetTextFieldStyle={vi.fn()}
|
||||
onAddTextField={vi.fn()}
|
||||
onRemoveTextField={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
});
|
||||
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<DomEditTextField[]>(makeMultiFieldElement().textFields);
|
||||
const element: DomEditSelection = { ...makeMultiFieldElement(), textFields: fields };
|
||||
return (
|
||||
<FlatTextSection
|
||||
element={element}
|
||||
styles={{}}
|
||||
fontAssets={[]}
|
||||
onSetText={vi.fn()}
|
||||
onSetTextFieldStyle={vi.fn()}
|
||||
onAddTextField={() =>
|
||||
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(<Harness />);
|
||||
});
|
||||
|
||||
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<HTMLButtonElement>('[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<HTMLButtonElement>(
|
||||
'[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());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string | null> | null;
|
||||
onRemoveTextField: (fieldKey: string) => void;
|
||||
}) {
|
||||
const [activeFieldKey, setActiveFieldKey] = useState<string | null>(
|
||||
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 (
|
||||
<TextSection
|
||||
element={element}
|
||||
styles={styles}
|
||||
fontAssets={fontAssets}
|
||||
onImportFonts={onImportFonts}
|
||||
onSetText={onSetText}
|
||||
onSetTextFieldStyle={onSetTextFieldStyle}
|
||||
onAddTextField={onAddTextField}
|
||||
onRemoveTextField={onRemoveTextField}
|
||||
hideOwnHeading
|
||||
/>
|
||||
<div className="space-y-1.5">
|
||||
<FlatTextLayerList
|
||||
fields={textFields}
|
||||
activeFieldKey={activeField.key}
|
||||
styles={styles}
|
||||
onSelect={setActiveFieldKey}
|
||||
onAdd={() =>
|
||||
void Promise.resolve(onAddTextField(activeField.key)).then((nextKey) => {
|
||||
if (nextKey) setActiveFieldKey(nextKey);
|
||||
})
|
||||
}
|
||||
onRemove={onRemoveTextField}
|
||||
/>
|
||||
<FlatTextFieldEditor
|
||||
field={activeField}
|
||||
styles={styles}
|
||||
fontAssets={fontAssets}
|
||||
onImportFonts={onImportFonts}
|
||||
onSetText={onSetText}
|
||||
onSetTextFieldStyle={onSetTextFieldStyle}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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());
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user