From c00294154aa3980695e1fd943701cd0e72feb1c3 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 8 Sep 2026 16:56:41 -0400 Subject: [PATCH 1/2] feat(studio): the inspector slider and select row on the shared primitives FlatSlider drops its hand-rolled pointer-capture state machine and FlatSelectRow its native select; both wrap the design system's Slider and Select, so the flat inspector has no bare underlined row left and no control of its own. What the wrapper still owns is the write rate. This row's onCommit is the live canvas preview, so the shared Slider's continuous onPreview is throttled to one write per frame rather than dropped; the commit boundary flushes it and fires telemetry exactly once per drag. Right-click and Escape still abort a drag in flight and put the value back. A platform pointercancel now does the same through the shared Slider, which had no handler for it. Adds a ratcheted check that no new native select or hand-rolled range input appears under components/editor. --- .../components/editor/flatSelectHarness.ts | 55 ++ .../components/editor/nativeControls.test.ts | 69 ++ .../propertyPanelColorSecondary.test.tsx | 10 +- ...pertyPanelFlatColorGradingSection.test.tsx | 2 +- .../propertyPanelFlatEffectsSection.test.tsx | 11 +- .../propertyPanelFlatMediaSection.test.tsx | 37 +- .../propertyPanelFlatPrimitives.test.tsx | 860 ++++++------------ .../editor/propertyPanelFlatPrimitives.tsx | 302 ++---- .../editor/propertyPanelFlatSelectRow.tsx | 66 +- .../propertyPanelFlatStyleSections.test.tsx | 76 +- .../propertyPanelInputCoverage.test.tsx | 22 +- packages/studio/src/components/ui/Slider.tsx | 5 + 12 files changed, 558 insertions(+), 957 deletions(-) create mode 100644 packages/studio/src/components/editor/flatSelectHarness.ts create mode 100644 packages/studio/src/components/editor/nativeControls.test.ts diff --git a/packages/studio/src/components/editor/flatSelectHarness.ts b/packages/studio/src/components/editor/flatSelectHarness.ts new file mode 100644 index 0000000000..ed9b64241a --- /dev/null +++ b/packages/studio/src/components/editor/flatSelectHarness.ts @@ -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 `` 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", + "propertyPanelFlatTextSection.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 ` outside the ratchet", () => { + expect(filesMatching(/ { + 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(/ { act(() => { host - .querySelector('[role="slider"][aria-label="Saturation min"]') + .querySelector('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({ @@ -107,7 +107,7 @@ describe("PropertyPanelColorSecondary", () => { act(() => { host - .querySelector('[role="slider"][aria-label="Luma max"]') + .querySelector('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({ @@ -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('[role="slider"][aria-label="Hue"]'); + const hue = host.querySelector('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()); }); diff --git a/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx index 987bd4ee78..350d6d8295 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx @@ -48,7 +48,7 @@ function findRowByText( } function dragSliderTrack(row: Element, clientX: number, trackWidth: number) { - const track = row.querySelector('[data-flat-slider-track="true"]'); + const track = row.querySelector("[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 }), diff --git a/packages/studio/src/components/editor/propertyPanelFlatEffectsSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatEffectsSection.test.tsx index c16b6f232b..d5d8f4b039 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatEffectsSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatEffectsSection.test.tsx @@ -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; @@ -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()); }); @@ -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", { @@ -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(); - expect( - host.querySelector('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", diff --git a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx index fcfb626a01..c96ff586bc 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx @@ -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; @@ -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()); }); @@ -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 }), }); @@ -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 }), }); @@ -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 }), }); @@ -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(); @@ -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"); @@ -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()); }); diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx index 013fa4bacd..4d6b3f0607 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx @@ -321,8 +321,50 @@ describe("FlatGroupHeader", () => { }); }); +/* ------------------------------------------------------------------ */ +/* Shared harness for the two rows that now wrap a Base UI control. */ +/* ------------------------------------------------------------------ */ + +/** Base UI moves focus and mounts a popup a task later; happy-dom is no faster. */ +const settle = () => act(async () => void (await new Promise((r) => setTimeout(r, 0)))); + +/** happy-dom has no layout, so Base UI's pointer maths needs a rect given to it. */ +function stubRect(el: Element, width = 100) { + el.getBoundingClientRect = () => + ({ x: 0, y: 0, top: 0, left: 0, right: width, bottom: 24, width, height: 24 }) as DOMRect; +} + +function sliderControl(host: HTMLElement, width = 100) { + const control = host.querySelector("[data-slider-control]"); + if (!control) throw new Error("expected a slider control"); + stubRect(control, width); + return control; +} + +/** The thumb's own input: where the value, the range and the keyboard live. */ +function sliderInput(host: HTMLElement) { + const input = host.querySelector('input[type="range"]'); + if (!input) throw new Error("expected a slider input"); + return input; +} + +function fire(el: Element, type: string, init: MouseEventInit & { key?: string } = {}) { + const event = + init.key === undefined + ? new MouseEvent(type, { bubbles: true, ...init }) + : new KeyboardEvent(type, { bubbles: true, key: init.key }); + act(() => void el.dispatchEvent(event)); +} + +/** `buttons: 1` is not decoration: Base UI reads it to tell a live drag from a + * move whose pointerup another element swallowed. */ +const move = (clientX: number) => + act(() => void document.dispatchEvent(new MouseEvent("pointermove", { clientX, buttons: 1 }))); +const release = (clientX: number) => + act(() => void document.dispatchEvent(new MouseEvent("pointerup", { clientX, buttons: 0 }))); + describe("FlatSlider", () => { - it("renders the default tier with a dim knob at the correct position", () => { + it("shows the display value, tinted by tier", () => { const { host, root } = renderInto( { onCommit={vi.fn()} />, ); - const knob = host.querySelector('[data-flat-slider-knob="true"]'); - expect(knob).not.toBeNull(); - expect(knob?.className).toContain("bg-panel-text-4"); - expect(knob?.style.left).toBe("0%"); const value = host.querySelector('[data-flat-slider-value="true"]'); - expect(value?.className).toContain("text-panel-text-3"); expect(value?.textContent).toBe("0px"); + expect(value?.className).toContain("text-text-3"); + // The thumb reports the value to assistive tech, which the hand-rolled + // knob only did because it carried the aria attributes by hand. + expect(sliderInput(host).getAttribute("aria-valuenow")).toBe("0"); act(() => root.unmount()); }); - it("renders the explicitCustom tier with a filled track and bright knob", () => { + it("marks an explicitly set value with the bright tier", () => { const { host, root } = renderInto( { onCommit={vi.fn()} />, ); - const fill = host.querySelector('[data-flat-slider-fill="true"]'); - expect(fill?.style.width).toBe("100%"); - const knob = host.querySelector('[data-flat-slider-knob="true"]'); - expect(knob?.className).toContain("bg-white"); + const value = host.querySelector('[data-flat-slider-value="true"]'); + expect(value?.className).toContain("text-text-0"); + expect(sliderInput(host).getAttribute("aria-valuenow")).toBe("100"); act(() => root.unmount()); }); - it("commits a value on track click, proportional to click position", () => { + it("commits a value proportional to where the track was pressed", () => { const onCommit = vi.fn(); const { host, root } = renderInto( { onCommit={onCommit} />, ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 2, right: 200, bottom: 2 }), - }); - act(() => { - track.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 100 })); - track.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 100 })); - }); - expect(onCommit).toHaveBeenCalledWith(50); - act(() => root.unmount()); - }); + const control = sliderControl(host); - it("widens the click/drag hit area vertically beyond the thin visible line", () => { - const onCommit = vi.fn(); - const { host, root } = renderInto( - , - ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new MouseEvent("pointerdown", { bubbles: true, clientX: 20, clientY: 18 }), - ); - track.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 20 })); - }); - expect(onCommit).toHaveBeenCalledWith(10); - act(() => root.unmount()); - }); + fire(control, "pointerdown", { clientX: 50, button: 0 }); + release(50); - it("tracks the knob instantly on every pointermove during a drag (draft state)", () => { - const onCommit = vi.fn(); - const { host, root } = renderInto( - , - ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), - ); - }); - // Instant, un-throttled knob feedback via aria-valuenow (draft state) — - // this must update on every pointermove regardless of the commit throttle. - expect(track.getAttribute("aria-valuenow")).toBe("10"); - act(() => { - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 160, pointerId: 1 }), - ); - }); - expect(track.getAttribute("aria-valuenow")).toBe("80"); - act(() => { - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 100, pointerId: 1 }), - ); - }); - expect(track.getAttribute("aria-valuenow")).toBe("50"); - act(() => { - track.dispatchEvent(new PointerEvent("pointerup", { bubbles: true, pointerId: 1 })); - }); - act(() => root.unmount()); - }); - - it("throttles rapid drag commits to leading edge + final value on release, not every step", () => { - const onCommit = vi.fn(); - const { host, root } = renderInto( - , - ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - // pointerdown fires the leading-edge commit immediately — a live - // preview needs to move the instant the drag starts, not wait 40ms. - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), - ); - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 160, pointerId: 1 }), - ); - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 100, pointerId: 1 }), - ); - }); - // The leading-edge commit (10) fired; the rapid intermediate position (80) - // from the first pointermove never committed — it's within the 40ms - // throttle window, so only the trailing flush or the pointerup release - // gets to send the next value. - expect(onCommit).toHaveBeenCalledTimes(1); - expect(onCommit).toHaveBeenCalledWith(10); - act(() => { - // Real pointerup events always carry the pointer's true release position - // (matches the last pointermove) — the handler recomputes from this - // rather than trusting a possibly-stale `draft` closure. - track.dispatchEvent( - new PointerEvent("pointerup", { bubbles: true, clientX: 100, pointerId: 1 }), - ); - }); - // Release flushes immediately with the LAST position only. - expect(onCommit).toHaveBeenCalledTimes(2); - expect(onCommit).toHaveBeenNthCalledWith(2, 50); + expect(onCommit.mock.calls).toEqual([[50]]); act(() => root.unmount()); }); - it("ignores pointermove once a drag has ended (pointer capture released)", () => { - const onCommit = vi.fn(); - const { host, root } = renderInto( - , - ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), - ); - track.dispatchEvent(new PointerEvent("pointerup", { bubbles: true, pointerId: 1 })); - }); - onCommit.mockClear(); - act(() => { - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 160, pointerId: 1 }), - ); - }); - expect(onCommit).not.toHaveBeenCalled(); - act(() => root.unmount()); - }); - - it("still commits the release position when releasePointerCapture synchronously fires lostpointercapture (real-browser behavior happy-dom doesn't replicate)", () => { + it("writes twice for a drag from 10 to 40 across five intermediate moves, not once per move", () => { + // The row's onCommit IS the live canvas preview, so mid-drag writes are the + // point; what must not happen is one write per pointermove. Five moves get + // a leading-edge write and then the released value, because the rest land + // inside the same throttle window. const onCommit = vi.fn(); const { host, root } = renderInto( { onCommit={onCommit} />, ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - // Real browsers fire lostpointercapture SYNCHRONOUSLY, mid-call, when - // releasePointerCapture() is invoked — happy-dom does not replicate this, - // so patch it in to reproduce the exact reentrancy hazard onPointerUp - // must guard against. - const originalRelease = track.releasePointerCapture.bind(track); - track.releasePointerCapture = (pointerId: number) => { - originalRelease(pointerId); - track.dispatchEvent(new Event("lostpointercapture", { bubbles: true })); - }; - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 30, pointerId: 1 }), - ); - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerup", { bubbles: true, clientX: 80, pointerId: 1 }), - ); - }); - // The real release position (80), not a rollback to the pre-drag value (10) - // caused by onLostPointerCapture resyncing mid-handler. - expect(onCommit).toHaveBeenLastCalledWith(80); - expect(track.getAttribute("aria-valuenow")).toBe("80"); + const control = sliderControl(host); + + fire(control, "pointerdown", { clientX: 10, button: 0 }); + for (const x of [15, 20, 25, 30, 40]) move(x); + release(40); + + expect(onCommit).toHaveBeenLastCalledWith(40); + expect(onCommit.mock.calls.length).toBeLessThanOrEqual(2); act(() => root.unmount()); }); - it("Escape during a drag reverts to the pre-drag value and releases pointer capture, instead of leaving the last dragged-to position committed", () => { + /** Past the throttle window, so a queued write actually lands. */ + const past = () => act(async () => void (await new Promise((r) => setTimeout(r, 60)))); + + /** + * The three ways a drag is abandoned rather than finished. All three have to + * put back the value the drag started from, and the interesting part is that + * a mid-drag write has ALREADY applied an intermediate value to the document + * by then, so an abort that merely stopped listening would leave the canvas + * wherever the pointer happened to be. + */ + const aborts = [ + ["the right button", (control: HTMLElement) => fire(control, "contextmenu", { button: 2 })], + ["the platform cancelling the gesture", (c: HTMLElement) => fire(c, "pointercancel", {})], + ["Escape", (control: HTMLElement) => fire(control, "keydown", { key: "Escape" })], + ] as const; + + it.each(aborts)("reverts a drag in flight aborted with %s (KTD8)", async (_name, abort) => { const onCommit = vi.fn(); const { host, root } = renderInto( { onCommit={onCommit} />, ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 30, pointerId: 1 }), - ); - }); - // The leading-edge commit already applied the dragged-to value (30). - expect(onCommit).toHaveBeenLastCalledWith(30); - act(() => { - track.dispatchEvent( - new KeyboardEvent("keydown", { key: "Escape", bubbles: true, cancelable: true }), - ); - }); - expect(onCommit).toHaveBeenLastCalledWith(10); - expect(track.getAttribute("aria-valuenow")).toBe("10"); - expect(track.hasPointerCapture(1)).toBe(false); - // A subsequent pointermove for the now-released pointer must not resume - // the cancelled drag. - onCommit.mockClear(); - act(() => { - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 80, pointerId: 1 }), - ); - }); - expect(onCommit).not.toHaveBeenCalled(); + const control = sliderControl(host); + + fire(control, "pointerdown", { clientX: 10, button: 0 }); + move(60); + expect(onCommit.mock.calls).toEqual([[60]]); + + abort(control); + // The pointer is still down: the abort has to stop Base UI applying any + // further move, not merely reset the number once. + move(90); + release(90); + await past(); + + expect(sliderInput(host).value).toBe("10"); + expect(onCommit.mock.calls).toEqual([[60], [10]]); act(() => root.unmount()); }); - it("right-click (contextmenu) during a drag cancels it and reverts to the pre-drag value, instead of committing the last dragged-to position", () => { + it("steps with the arrow keys and clamps with Home and End", () => { const onCommit = vi.fn(); const { host, root } = renderInto( , ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 65, pointerId: 1 }), - ); - }); - expect(onCommit).toHaveBeenLastCalledWith(65); - const contextMenuEvent = new MouseEvent("contextmenu", { bubbles: true, cancelable: true }); - act(() => { - track.dispatchEvent(contextMenuEvent); - }); - expect(contextMenuEvent.defaultPrevented).toBe(true); - expect(onCommit).toHaveBeenLastCalledWith(10); - expect(track.getAttribute("aria-valuenow")).toBe("10"); - expect(track.hasPointerCapture(1)).toBe(false); + const input = sliderInput(host); + expect(input.min).toBe("0"); + expect(input.max).toBe("100"); + + fire(input, "keydown", { key: "ArrowRight" }); + expect(onCommit).toHaveBeenLastCalledWith(51); + fire(input, "keydown", { key: "Home" }); + expect(onCommit).toHaveBeenLastCalledWith(0); + fire(input, "keydown", { key: "End" }); + expect(onCommit).toHaveBeenLastCalledWith(100); act(() => root.unmount()); }); - it("a native pointercancel during a drag reverts to the pre-drag value, instead of leaving the last dragged-to position committed", () => { + it("ignores the committed value echoing back mid-drag, so the thumb does not snap", () => { const onCommit = vi.fn(); - const { host, root } = renderInto( - , - ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 65, pointerId: 1 }), + function Harness() { + const [value, setValue] = React.useState(10); + return ( + { + onCommit(next); + setValue(next); + }} + /> ); - }); - expect(onCommit).toHaveBeenLastCalledWith(65); - act(() => { - track.dispatchEvent(new PointerEvent("pointercancel", { bubbles: true, pointerId: 1 })); - }); - expect(onCommit).toHaveBeenLastCalledWith(10); - expect(track.getAttribute("aria-valuenow")).toBe("10"); - expect(track.hasPointerCapture(1)).toBe(false); + } + const { host, root } = renderInto(); + const control = sliderControl(host); + + fire(control, "pointerdown", { clientX: 30, button: 0 }); + expect(onCommit).toHaveBeenCalledWith(30); + move(60); + + // 60, not the 30 the parent echoed back a render later. + expect(sliderInput(host).value).toBe("60"); act(() => root.unmount()); }); }); @@ -814,7 +668,7 @@ describe("FlatSlider — Grade extensions", () => { act(() => root.unmount()); }); - it("never commits from a click released on a disabled slider", () => { + it("never commits from a press released on a disabled slider", () => { const onCommit = vi.fn(); const { host, root } = renderInto( { onCommit={onCommit} />, ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 50, pointerId: 1 }), - ); - track.dispatchEvent( - new PointerEvent("pointerup", { bubbles: true, clientX: 50, pointerId: 1 }), - ); - }); + const control = sliderControl(host, 200); + + fire(control, "pointerdown", { clientX: 50, button: 0 }); + release(50); + expect(onCommit).not.toHaveBeenCalled(); act(() => root.unmount()); }); @@ -867,8 +713,7 @@ describe("FlatSlider — Grade extensions", () => { act(() => root.unmount()); }); - it("a trailing throttled commit uses the current render's onCommit, not the one captured when it was scheduled", () => { - vi.useFakeTimers(); + it("a trailing throttled write uses the current render's onCommit, not the one captured when it was scheduled", async () => { const onCommitA = vi.fn(); const { host, root } = renderInto( { onCommit={onCommitA} />, ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - // Leading-edge commit fires synchronously with onCommitA (clientX 150 - // on a -100..100 track maps to 50, distinct from the initial value 0). - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 150, pointerId: 1 }), - ); - }); + const control = sliderControl(host, 200); + + // Leading edge: clientX 150 on a 200px, -100..100 track is 50. + fire(control, "pointerdown", { clientX: 150, button: 0 }); expect(onCommitA).toHaveBeenCalledTimes(1); - act(() => { - // Within the 40ms throttle window — queues a trailing commit (to 80, - // distinct from the just-committed 50) instead of firing immediately. - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 180, pointerId: 1 }), - ); - }); + // Inside the throttle window, so this queues rather than writes. + move(180); expect(onCommitA).toHaveBeenCalledTimes(1); - // Simulate the real-world race: something else causes this slider to - // re-render with a NEW onCommit closure before the queued timer fires - // (e.g. Grade's per-detail onCommit spreads the render-time whole - // grading object, so a different control committing in between produces - // a fresh closure). The stale closure must not win. + + // The real race: something else re-renders this row with a NEW onCommit + // closure before the queued write fires. Grade's per-detail onCommit + // spreads the render-time grading object, so a stale closure winning here + // would silently revert whatever changed in between. const onCommitB = vi.fn(); act(() => { root.render( @@ -921,18 +753,14 @@ describe("FlatSlider — Grade extensions", () => { />, ); }); - act(() => { - vi.advanceTimersByTime(45); - }); - expect(onCommitB).toHaveBeenCalledTimes(1); - expect(onCommitB).toHaveBeenCalledWith(80); + await act(async () => void (await new Promise((r) => setTimeout(r, 60)))); + + expect(onCommitB.mock.calls).toEqual([[80]]); expect(onCommitA).toHaveBeenCalledTimes(1); act(() => root.unmount()); - vi.useRealTimers(); }); - it("flushes a still-queued trailing commit on unmount instead of dropping it", () => { - vi.useFakeTimers(); + it("flushes a still-queued trailing write on unmount instead of dropping it", () => { const onCommit = vi.fn(); const { host, root } = renderInto( { onCommit={onCommit} />, ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), - ); - }); + const control = sliderControl(host, 200); + + fire(control, "pointerdown", { clientX: 20, button: 0 }); expect(onCommit).toHaveBeenCalledTimes(1); - act(() => { - // Queues a trailing commit that never gets to fire before unmount. - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 160, pointerId: 1 }), - ); - }); + // Queues a write that never gets to fire before the row goes away. + move(160); expect(onCommit).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + expect(onCommit).toHaveBeenCalledTimes(2); expect(onCommit).toHaveBeenNthCalledWith(2, 80); - vi.useRealTimers(); }); +}); - it("supports keyboard operation: focusable, arrow keys step, Home/End clamp to range", () => { - const onCommit = vi.fn(); - const { host, root } = renderInto( - , - ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - expect(track.getAttribute("tabindex")).toBe("0"); - expect(track.getAttribute("aria-valuemin")).toBe("0"); - expect(track.getAttribute("aria-valuemax")).toBe("100"); - act(() => { - track.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowRight", bubbles: true })); - }); - expect(onCommit).toHaveBeenLastCalledWith(51); - act(() => { - track.dispatchEvent(new KeyboardEvent("keydown", { key: "Home", bubbles: true })); - }); - expect(onCommit).toHaveBeenLastCalledWith(0); - act(() => root.unmount()); - }); +/* ------------------------------------------------------------------ */ +/* FlatSelectRow */ +/* ------------------------------------------------------------------ */ - it("ignores the committed prop echoing back mid-drag (no knob snap-back)", () => { - const onCommit = vi.fn(); - function Harness() { - const [value, setValue] = React.useState(10); - return ( - { - onCommit(next); - setValue(next); - }} - /> - ); - } - const { host, root } = renderInto(); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - act(() => { - // Leading-edge commit fires at 30 and echoes back through the parent's - // state — mid-drag, that echo must NOT reset the draft. - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 30, pointerId: 1 }), - ); - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointermove", { bubbles: true, clientX: 80, pointerId: 1 }), - ); - }); - // Draft tracks the pointer (80), not the stale committed echo (30). - expect(track.getAttribute("aria-valuenow")).toBe("80"); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerup", { bubbles: true, clientX: 80, pointerId: 1 }), - ); - }); - expect(onCommit).toHaveBeenLastCalledWith(80); - expect(track.getAttribute("aria-valuenow")).toBe("80"); - act(() => root.unmount()); - }); +function selectTrigger(host: HTMLElement) { + const trigger = host.querySelector('[role="combobox"]'); + if (!trigger) throw new Error("expected a select trigger"); + return trigger; +} - it("resets the dragging state on lostpointercapture even without a prior pointerup/pointercancel", () => { +/** Opens the popup and returns its options, which only exist while open. */ +async function openOptions(host: HTMLElement) { + act(() => selectTrigger(host).click()); + await settle(); + return [...document.querySelectorAll('[role="option"]')] as HTMLElement[]; +} + +async function choose(host: HTMLElement, text: string) { + const options = await openOptions(host); + const option = options.find((el) => el.textContent === text); + if (!option) throw new Error(`no option "${text}" among ${options.map((o) => o.textContent)}`); + act(() => option.click()); + await settle(); +} + +describe("FlatSelectRow", () => { + it("renders the default tier with no reset button", () => { const { host, root } = renderInto( - , ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 30, pointerId: 1 }), - ); - }); - expect(track.getAttribute("aria-valuenow")).toBe("30"); - act(() => { - // Capture lost WITHOUT a pointerup/pointercancel first — e.g. another - // element steals it, or the browser reclaims it for a scroll gesture. - track.dispatchEvent(new Event("lostpointercapture", { bubbles: true })); - }); - act(() => { - root.render( - , - ); - }); - // If lostpointercapture hadn't cleared the dragging flag, this external - // value change would be silently ignored (mid-drag echo suppression) - // forever — the knob would be stuck at 30. - expect(track.getAttribute("aria-valuenow")).toBe("99"); + expect(selectTrigger(host).textContent).toContain("visible"); + expect(host.querySelector('[data-flat-select-reset="true"]')).toBeNull(); act(() => root.unmount()); }); - it("resyncs immediately from the latest value on lostpointercapture, even when the value changed WHILE still dragging", () => { + it("boxes the trigger the way the row's other fields are boxed", () => { + // R10 and R8 together: the value carries its own boundary, and it is the + // same boundary the text fields wear, not a native control's. const { host, root } = renderInto( - , ); - const track = host.querySelector('[data-flat-slider-track="true"]'); - if (!track) throw new Error("expected a track element"); - Object.defineProperty(track, "getBoundingClientRect", { - value: () => ({ left: 0, width: 100, top: 0, height: 20, right: 100, bottom: 20 }), - }); - act(() => { - track.dispatchEvent( - new PointerEvent("pointerdown", { bubbles: true, clientX: 30, pointerId: 1 }), - ); - }); - expect(track.getAttribute("aria-valuenow")).toBe("30"); - // Value changes to 99 WHILE still dragging — the [value] sync effect - // must skip it (draggingRef is still true), so draft stays at 30. - act(() => { - root.render( - , - ); - }); - expect(track.getAttribute("aria-valuenow")).toBe("30"); - act(() => { - // Capture lost with NO further render afterward — if the resync - // depended on a subsequent [value] effect run rather than reading - // latestValueRef directly, this would leave the knob stuck at 30. - track.dispatchEvent(new Event("lostpointercapture", { bubbles: true })); - }); - expect(track.getAttribute("aria-valuenow")).toBe("99"); + const trigger = selectTrigger(host); + expect(trigger.className).toContain("border-border-input"); + expect(trigger.className).toContain("bg-input"); act(() => root.unmount()); - }); -}); -describe("FlatSelectRow", () => { - it("renders the default tier with no reset button", () => { - const { host, root } = renderInto( + // An explicitly set value tints that same box, and `cn` has to let the + // tint win over the base border rather than leave both classes standing. + const { host: custom, root: rootB } = renderInto( , ); - const select = host.querySelector("select"); - expect(select?.value).toBe("normal"); - expect(host.querySelector('[data-flat-select-reset="true"]')).toBeNull(); - act(() => root.unmount()); + const tinted = selectTrigger(custom); + expect(tinted.className).toContain("border-accent/30"); + expect(tinted.className).not.toContain("border-border-input"); + expect(tinted.className).toContain("text-accent"); + act(() => rootB.unmount()); }); it("renders the explicitCustom tier with a reset button and fires onReset", () => { const onReset = vi.fn(); const { host, root } = renderInto( , ); - const select = host.querySelector("select"); - expect(select?.className).toContain("text-accent"); const reset = host.querySelector('[data-flat-select-reset="true"]'); + expect(reset).not.toBeNull(); act(() => reset?.dispatchEvent(new MouseEvent("click", { bubbles: true }))); expect(onReset).toHaveBeenCalledTimes(1); act(() => root.unmount()); }); - it("disables the reset button (and gives the select an accessible name) when the row itself is disabled", () => { + it("disables the reset button, and names the trigger, when the row itself is disabled", () => { const onReset = vi.fn(); const { host, root } = renderInto( , ); - const select = host.querySelector("select"); - expect(select?.getAttribute("aria-label")).toBe("Shadow"); + const trigger = selectTrigger(host); + expect(trigger.getAttribute("aria-label")).toBe("Preset"); + expect(trigger.hasAttribute("disabled")).toBe(true); const reset = host.querySelector('[data-flat-select-reset="true"]'); expect(reset?.disabled).toBe(true); act(() => reset?.dispatchEvent(new MouseEvent("click", { bubbles: true }))); @@ -1204,87 +907,80 @@ describe("FlatSelectRow", () => { act(() => root.unmount()); }); - it("fires onChange when the select value changes", () => { + it("commits the chosen option's literal value", async () => { const onChange = vi.fn(); const { host, root } = renderInto( , ); - const select = host.querySelector("select"); - if (!select) throw new Error("expected a select"); - act(() => { - select.value = "hidden"; - select.dispatchEvent(new Event("change", { bubbles: true })); - }); - expect(onChange).toHaveBeenCalledWith("hidden"); + + await choose(host, "scroll"); + + expect(onChange.mock.calls).toEqual([["scroll"]]); act(() => root.unmount()); }); }); describe("FlatSelectRow — label/value options", () => { - it("renders distinct labels for entries with a different display label than value", () => { + it("renders distinct labels for entries with a different display label than value", async () => { + const onChange = vi.fn(); const { host, root } = renderInto( , ); - const select = host.querySelector("select"); - expect(select?.value).toBe("clean-studio"); - const options = Array.from(host.querySelectorAll("option")).map((o) => o.textContent); - expect(options).toEqual(["Neutral", "Clean Studio", "Bright Pop"]); + expect(selectTrigger(host).textContent).toContain("400 · Regular"); + + await choose(host, "600 · Semibold"); + + // The literal union value, not its display label. + expect(onChange.mock.calls).toEqual([["600"]]); act(() => root.unmount()); }); - it("still treats a bare string array as value===label (Plan 2 behavior unchanged)", () => { + it("still treats a bare string array as value===label", async () => { const { host, root } = renderInto( , ); - const options = Array.from(host.querySelectorAll("option")).map((o) => o.textContent); - expect(options).toEqual(["normal", "multiply", "screen"]); + const options = await openOptions(host); + expect(options.map((el) => el.textContent)).toEqual(["visible", "hidden", "clip"]); act(() => root.unmount()); }); - it("preserves a valid authored value outside the preset list instead of misrepresenting it as the first option", () => { - const onChange = vi.fn(); + it("preserves a valid authored value outside the preset list instead of dropping it", async () => { + // A `mix-blend-mode` this row does not offer as a preset still has to be + // displayable, or the row shows nothing and any choice silently overwrites + // a value the user never saw. const { host, root } = renderInto( , ); - const select = host.querySelector("select"); - // A native */ +/* FlatSelectRow — label/value row backed by the shared Select */ /* ------------------------------------------------------------------ */ export function FlatSelectRow({ @@ -23,7 +24,7 @@ export function FlatSelectRow({ label: string; /** Accessible name when a caller renders the visible label OUTSIDE this * row (label="" to avoid a duplicate) — e.g. Grade's "Preset" row, which - * shows its own label span and would otherwise leave the falls - // back to selectedIndex 0 when `value` matches no