Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions packages/studio/src/components/editor/flatSelectHarness.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Driving a `FlatSelectRow` from a test, now that it is a Base UI select and
* not a native one.
*
* A native `<select>` could be changed in one line: set `value`, dispatch
* `change`. A listbox cannot, because its options only exist while the popup is
* open and the popup mounts in a portal a task after the trigger is pressed. So
* the sequence lives here once instead of in each section's test file.
*
* Deliberately not named `*.test.*`: vitest collects by that suffix, and a
* helper module with no tests in it would fail collection.
*/
import { act } from "react";

/** Base UI mounts and unmounts the popup a task later; happy-dom is no faster. */
const settle = () => act(async () => void (await new Promise((r) => setTimeout(r, 0))));

export function flatSelectRow(host: HTMLElement, label: string) {
const rows = Array.from(host.querySelectorAll<HTMLElement>(".group"));
const row = rows.find((el) => el.querySelector("span")?.textContent === label);
if (!row) throw new Error(`expected a select row for "${label}"`);
const trigger = row.querySelector<HTMLElement>('[role="combobox"]');
if (!trigger) throw new Error(`expected a select trigger for "${label}"`);
const resetButton = row.querySelector<HTMLButtonElement>('[data-flat-select-reset="true"]');
return { row, trigger, resetButton };
}

/** Opens the popup and returns its options, which exist only while it is open. */
export async function openFlatSelect(host: HTMLElement, label: string): Promise<HTMLElement[]> {
const { trigger } = flatSelectRow(host, label);
act(() => trigger.click());
await settle();
return [...document.querySelectorAll<HTMLElement>('[role="option"]')];
}

/** Picks from a list already open, so a caller that inspected it first does not
* have to reopen the popup — pressing the trigger again would close it. */
export async function chooseOpenOption(options: HTMLElement[], optionText: string): Promise<void> {
const option = options.find((el) => el.textContent === optionText);
if (!option) {
throw new Error(
`no option "${optionText}", among ${JSON.stringify(options.map((el) => el.textContent))}`,
);
}
act(() => option.click());
await settle();
}

export async function chooseFlatSelectOption(
host: HTMLElement,
label: string,
optionText: string,
): Promise<void> {
await chooseOpenOption(await openFlatSelect(host, label), optionText);
}
68 changes: 68 additions & 0 deletions packages/studio/src/components/editor/nativeControls.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* R13: a hand-rolled range input or a native `<select>` under the inspector is
* a duplicate of a shared primitive that already exists.
*
* A ratchet rather than a flat ban, because the inspector sweep is cut by
* section family and the later families have not moved yet. The allowlist is
* the set of files that still carry one; a new offender fails the test because
* it is not in the list, and a converted file fails it until it is removed, so
* the list can only shrink. U12 generalises this into the lint rule.
*/
import { readdirSync, readFileSync } from "node:fs";
import path from "node:path";
import { describe, expect, it } from "vitest";

const EDITOR_DIR = __dirname;

/** Still native. Delete an entry when its section family moves to the shared primitive. */
const NATIVE_SELECT_ALLOWED = [
"AnimationCardParts.tsx",
"BlockParamsPanel.tsx",
"EaseParamFields.tsx",
"KeyframeEaseList.tsx",
"propertyPanelColorGradingControls.tsx",
"propertyPanelColorGradingSection.tsx",
"propertyPanelFill.tsx",
"propertyPanelFlatColorGradingSection.tsx",
"propertyPanelFxControls.tsx",
"propertyPanelSections.tsx",
];

const RANGE_INPUT_ALLOWED = [
"BlockParamsPanel.tsx",
"propertyPanelColorGradingSlider.tsx",
"propertyPanelColorWheels.tsx",
"propertyPanelFxControls.tsx",
"propertyPanelFxEqModule.tsx",
];

function sourceFiles() {
return readdirSync(EDITOR_DIR)
.filter((name) => name.endsWith(".tsx") && !name.includes(".test."))
.sort();
}

/** Only real markup counts: a prose mention of `<select>` in a comment is not a control. */
function filesMatching(pattern: RegExp): string[] {
return sourceFiles().filter((name) => {
const source = readFileSync(path.join(EDITOR_DIR, name), "utf8");
return source
.split("\n")
.some((line) => pattern.test(line) && !line.trimStart().startsWith("*"));
});
}

describe("duplicate controls under components/editor (R13)", () => {
it("leaves no native <select> outside the ratchet", () => {
expect(filesMatching(/<select\b/)).toEqual(NATIVE_SELECT_ALLOWED);
});

it("leaves no hand-rolled range input outside the ratchet", () => {
expect(filesMatching(/type="range"/)).toEqual(RANGE_INPUT_ALLOWED);
});

it("finds files at all, so the two checks above are not vacuous", () => {
expect(sourceFiles().length).toBeGreaterThan(20);
expect(filesMatching(/<select\b/).length).toBeGreaterThan(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe("PropertyPanelColorSecondary", () => {

act(() => {
host
.querySelector<HTMLElement>('[role="slider"][aria-label="Saturation min"]')
.querySelector<HTMLElement>('input[type="range"][aria-label="Saturation min"]')
?.dispatchEvent(new KeyboardEvent("keydown", { key: "End", bubbles: true }));
});
expect(onCommit.mock.calls.at(-1)?.[0]?.[0]?.key.saturation).toMatchObject({
Expand All @@ -107,7 +107,7 @@ describe("PropertyPanelColorSecondary", () => {

act(() => {
host
.querySelector<HTMLElement>('[role="slider"][aria-label="Luma max"]')
.querySelector<HTMLElement>('input[type="range"][aria-label="Luma max"]')
?.dispatchEvent(new KeyboardEvent("keydown", { key: "Home", bubbles: true }));
});
expect(onCommit.mock.calls.at(-1)?.[0]?.[0]?.key.luma).toMatchObject({
Expand All @@ -128,10 +128,12 @@ describe("PropertyPanelColorSecondary", () => {
});
if (!grading?.secondaries) throw new Error("Expected normalized secondaries");
const { host, root } = renderSecondary({ secondaries: grading.secondaries });
const hue = host.querySelector<HTMLElement>('[role="slider"][aria-label="Hue"]');
const hue = host.querySelector<HTMLElement>('input[type="range"][aria-label="Hue"]');

expect(Number(hue?.getAttribute("aria-valuenow"))).toBeCloseTo(359.7);
expect(hue?.getAttribute("aria-valuemax")).toBe("359.99");
// The thumb is a real range input now, so the bound is the native `max`
// rather than an aria attribute the hand-rolled track had to write itself.
expect(hue?.getAttribute("max")).toBe("359.99");
act(() => root.unmount());
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function findRowByText(
}

function dragSliderTrack(row: Element, clientX: number, trackWidth: number) {
const track = row.querySelector<HTMLElement>('[data-flat-slider-track="true"]');
const track = row.querySelector<HTMLElement>("[data-slider-control]");
if (!track) throw new Error("expected a slider track");
Object.defineProperty(track, "getBoundingClientRect", {
value: () => ({ left: 0, width: trackWidth, top: 0, height: 2, right: trackWidth, bottom: 2 }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
FlatEffectsSection,
} from "./propertyPanelFlatEffectsSection";
import { EFFECT_SPECS } from "./propertyPanelFlatEffectSpecs";
import { openFlatSelect } from "./flatSelectHarness";

(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;

Expand Down Expand Up @@ -111,7 +112,7 @@ describe("FlatEffectsSection", () => {
for (const effect of EFFECT_SPECS) {
expect(Boolean(effect.palette)).toBe(capabilities.get(effect.key)?.supportsPalette);
}
expect(host.querySelectorAll('[data-flat-slider-track="true"]')).toHaveLength(0);
expect(host.querySelectorAll("[data-slider-control]")).toHaveLength(0);
act(() => root.unmount());
});

Expand Down Expand Up @@ -190,7 +191,7 @@ describe("FlatEffectsSection", () => {
expect(host.textContent).toContain("Angle");
const effect = host.querySelector('[data-flat-effect-editor="chromaticAberration"]');
if (!effect) throw new Error("expected chromatic effect");
const rows = effect.querySelectorAll('[data-flat-slider-track="true"]');
const rows = effect.querySelectorAll("[data-slider-control]");
const angleTrack = rows[1] as HTMLElement | undefined;
if (!angleTrack) throw new Error("expected angle slider");
Object.defineProperty(angleTrack, "getBoundingClientRect", {
Expand All @@ -204,16 +205,14 @@ describe("FlatEffectsSection", () => {
act(() => root.unmount());
});

it("offers native ASCII styles, binary controls, and a bounded custom palette", () => {
it("offers native ASCII styles, binary controls, and a bounded custom palette", async () => {
const onCommit = vi.fn();
const grading = normalizeHfColorGrading({
effects: HF_COLOR_GRADING_EFFECT_APPLY_DEFAULTS.ascii,
});
if (!grading) throw new Error("expected ASCII grading");
const { host, root } = renderInto(<FlatEffectsSection {...sectionProps(grading, onCommit)} />);
expect(
host.querySelector<HTMLSelectElement>('select[aria-label="Style"]')?.options,
).toHaveLength(8);
expect(await openFlatSelect(host, "Style")).toHaveLength(8);
expect(host.querySelectorAll('[role="switch"]')).toHaveLength(2);
expect(host.querySelector('[data-flat-effect-editor="ascii"]')?.textContent).not.toContain(
"Mix",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import { FlatMediaSection } from "./propertyPanelFlatMediaSection";
import type { DomEditSelection } from "./domEditing";
import { chooseFlatSelectOption, flatSelectRow } from "./flatSelectHarness";

(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;

Expand Down Expand Up @@ -150,9 +151,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => {
);
});
expect(host.textContent).toContain("0.0 dB");
expect(
host.querySelector('[data-flat-slider-track="true"]')?.getAttribute("aria-valuenow"),
).toBe("0");
expect(host.querySelector('input[type="range"]')?.getAttribute("aria-valuenow")).toBe("0");
act(() => root.unmount());
});

Expand All @@ -174,7 +173,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => {
/>,
);
});
const volumeTrack = host.querySelectorAll('[data-flat-slider-track="true"]')[0];
const volumeTrack = host.querySelectorAll("[data-slider-control]")[0];
Object.defineProperty(volumeTrack, "getBoundingClientRect", {
value: () => ({ left: 0, width: 100, top: 0, height: 2, right: 100, bottom: 2 }),
});
Expand Down Expand Up @@ -207,7 +206,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => {
/>,
);
});
const rateTrack = host.querySelectorAll('[data-flat-slider-track="true"]')[1];
const rateTrack = host.querySelectorAll("[data-slider-control]")[1];
Object.defineProperty(rateTrack, "getBoundingClientRect", {
value: () => ({ left: 0, width: 100, top: 0, height: 2, right: 100, bottom: 2 }),
});
Expand Down Expand Up @@ -238,7 +237,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => {
/>,
);
});
const mediaStartTrack = host.querySelectorAll('[data-flat-slider-track="true"]')[2];
const mediaStartTrack = host.querySelectorAll("[data-slider-control]")[2];
Object.defineProperty(mediaStartTrack, "getBoundingClientRect", {
value: () => ({ left: 0, width: 100, top: 0, height: 2, right: 100, bottom: 2 }),
});
Expand Down Expand Up @@ -372,7 +371,7 @@ describe("FlatMediaSection — loop/muted/has-audio", () => {
});

describe("FlatMediaSection — fit/position", () => {
it("commits object-fit and object-position changes", () => {
it("commits object-fit and object-position changes", async () => {
const onSetStyle = vi.fn();
const { host, root } = (() => {
const element = makeVideoElement();
Expand All @@ -393,20 +392,13 @@ describe("FlatMediaSection — fit/position", () => {
});
return { host, root };
})();
const selects = host.querySelectorAll("select");
const fitSelect = Array.from(selects).find((s) => s.value === "cover");
expect(fitSelect).not.toBeUndefined();
act(() => {
if (fitSelect) {
fitSelect.value = "contain";
fitSelect.dispatchEvent(new Event("change", { bubbles: true }));
}
});
expect(flatSelectRow(host, "Fit").trigger.textContent).toContain("cover");
await chooseFlatSelectOption(host, "Fit", "contain");
expect(onSetStyle).toHaveBeenCalledWith("object-fit", "contain");
act(() => root.unmount());
});

it("commits an object-position change", () => {
it("commits an object-position change", async () => {
const onSetStyle = vi.fn();
const element = makeVideoElement();
const host = document.createElement("div");
Expand All @@ -424,15 +416,8 @@ describe("FlatMediaSection — fit/position", () => {
/>,
);
});
const selects = host.querySelectorAll("select");
const positionSelect = Array.from(selects).find((s) => s.value === "center");
expect(positionSelect).not.toBeUndefined();
act(() => {
if (positionSelect) {
positionSelect.value = "left top";
positionSelect.dispatchEvent(new Event("change", { bubbles: true }));
}
});
expect(flatSelectRow(host, "Position").trigger.textContent).toContain("center");
await chooseFlatSelectOption(host, "Position", "left top");
expect(onSetStyle).toHaveBeenCalledWith("object-position", "left top");
act(() => root.unmount());
});
Expand Down
Loading
Loading