diff --git a/src/components/_shared/overlayPosition.ts b/src/components/_shared/overlayPosition.ts index 5e3ea960..8fc472a4 100644 --- a/src/components/_shared/overlayPosition.ts +++ b/src/components/_shared/overlayPosition.ts @@ -103,16 +103,18 @@ export const resolveOverlayAnchorRect = ( export const createOverlayPosition = ( options: CreateOverlayPositionOptions, ) => { - const [style, setStyle] = createSignal({ - visibility: "hidden", - }); + // Visibility belongs to each overlay component's `data-open` CSS contract. + // Positioning must never write an inline visibility override: portal refs can + // settle after the open signal, and an override left behind here makes a + // semantically open Select or Popover impossible to see or operate. + const [style, setStyle] = createSignal({}); const [resolvedPlacement, setResolvedPlacement] = createSignal(options.placement()); createTrackedEffect(() => { if (!options.open()) { setResolvedPlacement(options.placement()); - setStyle({ visibility: "hidden" }); + setStyle({}); return; } @@ -215,10 +217,8 @@ export const createOverlayPosition = ( frame = requestAnimationFrame(update); }; - // Opening must make the overlay paint in the same reactive turn. Keeping - // the initial inline `visibility: hidden` until requestAnimationFrame - // overrides the component's `[data-open="true"]` CSS and leaves a valid - // select or popover invisible whenever the next frame is delayed. + // Opening must position the overlay in the same reactive turn. Visibility + // is intentionally absent from this style object; data-open CSS owns it. update(); window.addEventListener("resize", schedule); diff --git a/tests/components/popover/Popover.anchor.test.ts b/tests/components/popover/Popover.anchor.test.ts index 96b865d4..303bd064 100644 --- a/tests/components/popover/Popover.anchor.test.ts +++ b/tests/components/popover/Popover.anchor.test.ts @@ -49,7 +49,7 @@ describe("Popover virtual anchoring", () => { expect(resolveOverlayAnchorRect(trigger, undefined)).toBe(triggerRect); }); - it("removes the hidden inline style before the next animation frame", async () => { + it("never overrides the component visibility contract", async () => { const requestFrame = mock(() => 1); Object.defineProperty(globalThis, "window", { configurable: true, @@ -90,7 +90,7 @@ describe("Popover virtual anchoring", () => { offset: () => 6, }); - expect(position.style()).toEqual({ visibility: "hidden" }); + expect(position.style()).toEqual({}); setOpen(true); await Promise.resolve(); expect(position.style()).toMatchObject({ @@ -103,4 +103,61 @@ describe("Popover virtual anchoring", () => { dispose(); }); }); + + it("stays visibility-safe while a portal ref settles after opening", async () => { + Object.defineProperty(globalThis, "window", { + configurable: true, + value: { + innerWidth: 800, + innerHeight: 600, + addEventListener: mock(() => {}), + removeEventListener: mock(() => {}), + }, + }); + Object.defineProperty(globalThis, "requestAnimationFrame", { + configurable: true, + value: mock(() => 1), + }); + Object.defineProperty(globalThis, "cancelAnimationFrame", { + configurable: true, + value: mock(() => {}), + }); + + await createRoot(async (dispose) => { + const [open, setOpen] = createSignal(false); + const [overlay, setOverlay] = createSignal(); + const position = createOverlayPosition({ + open, + triggerRef: () => undefined, + anchorRect: () => virtualRect, + overlayRef: overlay, + placement: () => "bottom", + offset: () => 6, + }); + + setOpen(true); + await Promise.resolve(); + expect(position.style()).toEqual({}); + + setOverlay({ + getBoundingClientRect: () => ({ + top: 0, + left: 0, + width: 160, + height: 120, + right: 160, + bottom: 120, + }), + } as HTMLElement); + await Promise.resolve(); + + expect(position.style()).toMatchObject({ + position: "fixed", + top: "56px", + left: "8px", + }); + expect(position.style()).not.toHaveProperty("visibility"); + dispose(); + }); + }); }); diff --git a/tests/qa-harness/rsbuild.config.ts b/tests/qa-harness/rsbuild.config.ts index 57cef283..5beb99de 100644 --- a/tests/qa-harness/rsbuild.config.ts +++ b/tests/qa-harness/rsbuild.config.ts @@ -6,7 +6,6 @@ * the working tree rather than the last publish. */ import { defineConfig } from "@rsbuild/core"; -import { pluginSolid } from "@rsbuild/plugin-solid"; import { pluginBabel } from "@rsbuild/plugin-babel"; import { pluginSolidLayoutsApplication } from "rsbuild-plugin-solid-layouts"; import { resolve } from "node:path"; @@ -19,13 +18,27 @@ export default defineConfig({ // compiles `*.layout.tsx` sources; this one wires `solid-layouts` for a // consumer, which is what the harness is. pluginSolidLayoutsApplication(), - pluginBabel({ include: /\.(?:jsx|tsx)$/ }), - // `moduleName`/`generate` for the same reason `rslib.config.ts` sets them: - // `@rsbuild/plugin-solid` resolves a nested Solid 1 preset whose transform - // emits `solid-js/web`, a subpath Solid 2 dropped. Without this the harness - // builds and then fails to resolve at runtime. - pluginSolid({ - solidPresetOptions: { moduleName: "@solidjs/web", generate: "dom" }, + /* + * Babel does the Solid transform; `@rsbuild/plugin-solid` is deliberately + * absent. It injects solid-refresh, whose `$component` wrapper calls + * `createSignal(component)` — and Solid 2 reads a function initialiser as a + * derivation, so it builds a computed that needs an owner. Every harness + * page rendered an empty body and reported "Cannot read properties of + * undefined (reading 'spec')": the component was being invoked with no + * props at all. + * + * `moduleName` stays for the original reason — the transform must emit + * `@solidjs/web`, since Solid 2 dropped the `solid-js/web` subpath. + */ + pluginBabel({ + include: /\.(?:jsx|tsx)$/, + babelLoaderOptions: (config) => { + config.presets ??= []; + config.presets.push([ + "babel-preset-solid", + { moduleName: "@solidjs/web", generate: "dom" }, + ]); + }, }), ], /*