From 34f413439bb1bfe765b3c3722e11e614c72e04c2 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 8 Sep 2026 05:06:14 +0700 Subject: [PATCH] fix(color-wheel-flower): render standalone instead of crashing the page `ColorWheelFlower` is exported from `@pathscale/ui/lab`. Rendering one outside a `ThemeColorPicker` threw, and in Solid 2 a throw during render halts the reactive system: one component on one route blanked an entire application. The showcase page for it in js.software was dead, with `ContextNotFoundError` and `REACTIVITY_HALTED` in the console and an empty body. Two things had to be true for that. The context was declared `createContext(undefined)`. In Solid 2 that is the *default-less* form, and the absence of a default is precisely what makes `useContext` throw. It was paired with a hook that checked for a missing value and threw a friendlier error, which could never run because `useContext` threw first -- and whose message named `ColorPickerContext.Provider`, which Solid 2 does not have. The default is now `null` and the hook returns it. The component then dereferenced that context unconditionally. It now falls back to its own props: `color` / `defaultColor` for the selection, `disabled`, and `onChange`. Inside a `ThemeColorPicker` nothing changes, the surrounding context still owns the state. Outside one, a bare `` renders an uncontrolled flower starting at white. `tests/components/optional-context-defaults.test.ts` did not cover this on purpose: it exempts contexts that are dereferenced directly, on the argument that a default would trade a clear error for a null-property crash. That argument holds for an internal subcomponent and not for an exported one, and this was exported. The verifier is a ps-qa check, because the thing to prove is that it renders. It asserts the centre petal by name rather than the component name, which the harness renders on a labelled wrapper whether or not anything mounted -- the first version of this check passed against the unfixed component, which is how that was found. With the context restored to default-less it fails: no node matching "radio:Reset to neutral" exists in the tree --- docs/api-contract.md | 4 ++ .../ColorWheelFlower.layout.tsx | 60 ++++++++++++++++++- .../colorWheelFlowerContext.ts | 39 ++++++++---- tests/ps-qa-headless/color-wheel-flower.ron | 37 ++++++++++++ tests/ps-qa/color-wheel-flower.ron | 37 ++++++++++++ tests/qa-harness/components.ts | 30 ++++++++++ tests/qa-harness/generate-entries.ts | 2 + 7 files changed, 196 insertions(+), 13 deletions(-) create mode 100644 tests/ps-qa-headless/color-wheel-flower.ron create mode 100644 tests/ps-qa/color-wheel-flower.ron diff --git a/docs/api-contract.md b/docs/api-contract.md index 0353b649..daf9761f 100644 --- a/docs/api-contract.md +++ b/docs/api-contract.md @@ -446,8 +446,12 @@ wheelClass?: string ```ts class?: string +color?: ColorValue | string +defaultColor?: ColorValue | string +disabled?: boolean id?: string mode?: ColorWheelFlowerMode +onChange?: (color: ColorValue) => void palette?: readonly string[] ``` diff --git a/src/components/color-wheel-flower/ColorWheelFlower.layout.tsx b/src/components/color-wheel-flower/ColorWheelFlower.layout.tsx index a84fa3ad..7b39f6e9 100644 --- a/src/components/color-wheel-flower/ColorWheelFlower.layout.tsx +++ b/src/components/color-wheel-flower/ColorWheelFlower.layout.tsx @@ -5,7 +5,10 @@ import { clsx } from "clsx"; import { twMerge } from "../../lib/twMerge"; import ColorSwatch from "../color-swatch"; import ColorSwatchPicker from "../color-swatch-picker"; -import { useColorPickerContext } from "./colorWheelFlowerContext"; +import { + type ColorPickerContextType, + useColorPickerContext, +} from "./colorWheelFlowerContext"; import { createColorFromHsl, parseColor, @@ -31,6 +34,23 @@ export interface ColorWheelFlowerProps { mode?: ColorWheelFlowerMode; /** Exactly 31 literal colors, ordered outer ring, middle ring, inner ring, center. */ palette?: readonly string[]; + /* + * Standalone use. Inside a `ThemeColorPicker` the surrounding context owns + * the colour and these are ignored; outside one they are the whole state, + * and omitting all of them still renders -- an uncontrolled flower starting + * at white. + * + * They exist because this component is exported from `@pathscale/ui/lab`, + * and an exported component that can only be rendered inside one specific + * parent is a trap. It used to be a crashing one. + */ + /** Controlled selection. A hex, `rgb()` or `hsl()` string, or a parsed value. */ + color?: ColorValue | string; + /** Initial selection when uncontrolled. */ + defaultColor?: ColorValue | string; + disabled?: boolean; + /** The colour a petal was clicked to choose. */ + onChange?: (color: ColorValue) => void; } type ColorItem = { @@ -173,7 +193,43 @@ function buildColors(palette: readonly string[]): ColorItem[] { const CENTER_INDEX = LAYOUT.findIndex((l) => l.isCenter); const ColorWheelFlower: Layout = () => { - const context = useColorPickerContext(); + /* + * The surrounding picker if there is one, this component's own props if not. + * + * `useColorPickerContext()` returns `null` outside a `ThemeColorPicker`, and + * the fallback below is what makes that a supported way to use the flower + * rather than a crash. The shapes are identical, so nothing downstream has + * to know which one it got. + */ + const providedContext = useColorPickerContext(); + + const toColorValue = ( + value: ColorValue | string | undefined, + ): ColorValue | null => + value === undefined + ? null + : typeof value === "string" + ? parseColor(value) + : value; + + const WHITE = createColorFromHsl(0, 0, 100, 1); + + const [standaloneColor, setStandaloneColor] = createSignal( + toColorValue(props.defaultColor) ?? WHITE, + ); + + const context: ColorPickerContextType = providedContext ?? { + // A `color` prop makes it controlled; without one the click below is what + // moves the selection. + color: () => toColorValue(props.color) ?? standaloneColor(), + format: () => "hex" as const, + disabled: () => Boolean(props.disabled), + onChange: (next: ColorValue) => { + if (props.color === undefined) setStandaloneColor(next); + props.onChange?.(next); + }, + onFormatChange: () => {}, + }; const [selectedIndex, setSelectedIndex] = createSignal(null); const [pulseState, setPulseState] = createSignal<{ diff --git a/src/components/color-wheel-flower/colorWheelFlowerContext.ts b/src/components/color-wheel-flower/colorWheelFlowerContext.ts index e74e189d..4bca395d 100644 --- a/src/components/color-wheel-flower/colorWheelFlowerContext.ts +++ b/src/components/color-wheel-flower/colorWheelFlowerContext.ts @@ -9,16 +9,33 @@ export interface ColorPickerContextType { onFormatChange: (format: ColorFormat) => void; } -export const ColorPickerContext = createContext< - ColorPickerContextType | undefined ->(undefined); +/* + * The default is `null`, and it has to be something. + * + * Solid 2 treats `createContext(undefined)` as the *default-less* form: the + * absence of a default is what makes `useContext` throw `ContextNotFoundError` + * outside a provider. This context was declared `createContext(undefined)` + * and paired with a hook that checked for a missing value and threw a friendly + * message -- a check that could never run, because `useContext` threw first. + * The friendly message also named `ColorPickerContext.Provider`, which Solid 2 + * does not have. + * + * What that cost: `ColorWheelFlower` is exported from `@pathscale/ui/lab`, and + * rendering one outside a `ThemeColorPicker` did not degrade or warn. It threw + * during render, which in Solid 2 halts the reactive system for the whole + * page. One component on one route took an entire application down. + */ +export const ColorPickerContext = createContext( + null, +); -export function useColorPickerContext(): ColorPickerContextType { - const context = useContext(ColorPickerContext); - if (!context) { - throw new Error( - "useColorPickerContext must be used within a ColorPickerContext.Provider", - ); - } - return context; +/** + * The surrounding picker's state, or `null` when there is no picker. + * + * Returning `null` rather than throwing is deliberate: a consumer that can + * stand alone decides for itself what to do without a provider, and one that + * genuinely cannot say so in its own words. + */ +export function useColorPickerContext(): ColorPickerContextType | null { + return useContext(ColorPickerContext); } diff --git a/tests/ps-qa-headless/color-wheel-flower.ron b/tests/ps-qa-headless/color-wheel-flower.ron new file mode 100644 index 00000000..e516b4c8 --- /dev/null +++ b/tests/ps-qa-headless/color-wheel-flower.ron @@ -0,0 +1,37 @@ +// Generated from tests/qa-harness/components.ts by tests/qa-harness/generate-checks.ts. Do not edit. +// +// ColorWheelFlower, mounted alone on its own harness page. Outcomes for +// this component share one native host; idempotent preparation keeps each +// outcome reproducible by id against a fresh host as well. +[ + ( + id: "color-wheel-flower-page-paints", + group: "color-wheel-flower", + what: "the ColorWheelFlower page builds and paints", + open: None, + hover: None, + click: None, + subject: "heading:ColorWheelFlower", + expect: Present, + ), + ( + id: "color-wheel-flower-renders", + group: "color-wheel-flower", + what: "ColorWheelFlower renders a node of its own", + open: None, + hover: None, + click: None, + subject: "fixture", + expect: Paints, + ), + ( + id: "color-wheel-flower-paints", + group: "color-wheel-flower", + what: "the ColorWheelFlower reaches the renderer with a box", + open: None, + hover: None, + click: None, + subject: "radio:Reset to neutral", + expect: Present, + ), +] diff --git a/tests/ps-qa/color-wheel-flower.ron b/tests/ps-qa/color-wheel-flower.ron new file mode 100644 index 00000000..c8c87bf9 --- /dev/null +++ b/tests/ps-qa/color-wheel-flower.ron @@ -0,0 +1,37 @@ +// Generated from tests/qa-harness/components.ts by tests/qa-harness/generate-checks.ts. Do not edit. +// +// ColorWheelFlower, mounted alone on its own harness page. Outcomes for +// this component share one native host; idempotent preparation keeps each +// outcome reproducible by id against a fresh host as well. +[ + ( + id: "color-wheel-flower-page-paints", + group: "color-wheel-flower", + what: "the ColorWheelFlower page builds and paints", + open: None, + hover: None, + click: None, + subject: "heading:ColorWheelFlower", + expect: PaintsNamed, + ), + ( + id: "color-wheel-flower-renders", + group: "color-wheel-flower", + what: "ColorWheelFlower renders a node of its own", + open: None, + hover: None, + click: None, + subject: "fixture", + expect: Paints, + ), + ( + id: "color-wheel-flower-paints", + group: "color-wheel-flower", + what: "the ColorWheelFlower reaches the renderer with a box", + open: None, + hover: None, + click: None, + subject: "radio:Reset to neutral", + expect: PaintsNamed, + ), +] diff --git a/tests/qa-harness/components.ts b/tests/qa-harness/components.ts index f78c05e7..4c100c9e 100644 --- a/tests/qa-harness/components.ts +++ b/tests/qa-harness/components.ts @@ -237,6 +237,36 @@ export const COMPONENTS: ComponentSpec[] = [ subject: "Color undefined", subjectRole: "option", }, + /* + * The flower on its own, with no `ThemeColorPicker` around it. + * + * That is the arrangement that used to throw. It is exported from + * `@pathscale/ui/lab`, so a reader can write exactly this, and it read a + * context declared `createContext(undefined)` -- the default-less form in + * Solid 2, which throws `ContextNotFoundError` before the component's own + * "you must use this inside a provider" guard can run. The throw halted the + * reactive system and blanked the page it was on. + * + * `complex-color-wheel` did not cover it: that fixture mounts the flower + * under a wheel that supplies the context, which is the case that always + * worked. + */ + { + id: "color-wheel-flower", + component: "ColorWheelFlower", + kind: "display", + /* + * The centre petal, by name. + * + * Not the component name: the fixture renders that on a labelled wrapper, + * so it is there whether or not the component rendered anything, and a + * check that asserts it passes against a component that threw. This name + * comes from inside the flower, so nothing paints it unless the flower + * built its palette. + */ + subject: "Reset to neutral", + subjectRole: "radio", + }, { id: "color-wheel", component: "ColorWheel", diff --git a/tests/qa-harness/generate-entries.ts b/tests/qa-harness/generate-entries.ts index d2bbc263..dee038b7 100644 --- a/tests/qa-harness/generate-entries.ts +++ b/tests/qa-harness/generate-entries.ts @@ -78,6 +78,7 @@ const IMPORT_FORM: Record = { "ConnectionSettings": "named", "ColorSwatch": "default", "ColorWheel": "named", + "ColorWheelFlower": "named", "ComplexColorWheel": "named", "autosize": "named", "boundsFromRows": "named", @@ -257,6 +258,7 @@ const MODULE_PATHS: Record = { "connection-settings": "components/connection-settings", "color-swatch": "components/color-swatch", "color-wheel": "components/color-wheel", + "color-wheel-flower": "components/color-wheel-flower", "complex-color-wheel": "components/color-wheel", "composer": "components/composer", "cookie-consent": "components/immersive-landing",