diff --git a/.changeset/spinner-themeable-diameter-rail.md b/.changeset/spinner-themeable-diameter-rail.md new file mode 100644 index 0000000000000..8296a6bb358c5 --- /dev/null +++ b/.changeset/spinner-themeable-diameter-rail.md @@ -0,0 +1,20 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] Spinner: the ring's geometry and its two colors are now themeable. The `size` and `shade` props keep their fixed enums; what each named value _resolves to_ is now a theme's to set, through four public custom properties on the `spinner` target — `--spinner-diameter` and `--spinner-stroke-width` under a size variant, `--spinner-color` and `--spinner-track-color` under a shade variant (or on the base target for all of them at once): + +```ts +spinner: { + 'size:xl': {'--spinner-diameter': '2.5rem', '--spinner-stroke-width': '0.375rem'}, + 'shade:subtle': {'--spinner-track-color': 'transparent'}, +} +``` + +Any length and any color notation works — `rem`, `em` and `calc()` are resolved by the cascade into the radius and stroke the ring is drawn with, and colors accept `var()`, `color-mix()` and `currentColor`. A stroke width of `0` is honoured as a zero-width stroke — it paints nothing, rather than being read as "unset" and silently drawing the default. The drawn ring and the box around it come from the same values, so they stay in step, including when a media query or a root font-size change moves them after mount. + +The two private vars the ring resolves into are registered as `` when the module is imported, not when a spinner first mounts. Registering an inherited property with an `initial-value` invalidates style for the whole document, and a spinner is the loading indicator — it arrives on a page that has already rendered, so paying that there is paying it on the full tree: 29 ms against 12 ms for the same mount on an 11k-element page. A build that never imports `Spinner` drops the module and the registration with it. + +Output is unchanged for every size and shade unless a theme overrides something, and so is every precedence around the box: it is still sized by an inline `width`/`height` written after the caller's `style`, as it has always been, with the composed value in place of the number. + +@freddymeta diff --git a/apps/storybook/stories/Spinner.stories.tsx b/apps/storybook/stories/Spinner.stories.tsx index d75b51d7e874d..e4e8fb263ccff 100644 --- a/apps/storybook/stories/Spinner.stories.tsx +++ b/apps/storybook/stories/Spinner.stories.tsx @@ -4,6 +4,7 @@ import type {Meta, StoryObj} from '@storybook/react'; import {Spinner} from '@astryxdesign/core/Spinner'; import {Text} from '@astryxdesign/core/Text'; import {HStack, VStack} from '@astryxdesign/core/Layout'; +import {Theme, defineTheme} from '@astryxdesign/core/theme'; const meta: Meta = { title: 'Core/Spinner', @@ -17,7 +18,7 @@ const meta: Meta = { }, shade: { control: 'select', - options: ['default', 'onMedia'], + options: ['default', 'onMedia', 'subtle', 'inherit'], description: 'Color shade', }, }, @@ -81,3 +82,129 @@ export const WithLabel: Story = { ), }; + +// The ring is an SVG circle whose radius and stroke come off the cascade, so a +// theme reaches its geometry and its two colors through public custom +// properties rather than CSS box properties — `width` would name a box the +// ring is not. These stories are how that surface is checked: the a11y and RTL +// audits only see what a story renders, and a themed ring cannot be verified +// in jsdom (no layout, no registered properties), so this is where a browser +// can look at it. +// +// Geometry is deliberately themed in `rem` rather than `px`: the resolved vars +// are registered as ``, and a relative unit surviving into the drawn +// radius is what distinguishes a resolved value from substituted text. +const themedGeometry = defineTheme({ + name: 'spinner-themed-geometry', + components: { + spinner: { + 'size:sm': { + '--spinner-diameter': '1rem', + '--spinner-stroke-width': '0.125rem', + }, + 'size:md': { + '--spinner-diameter': '1.5rem', + '--spinner-stroke-width': '0.25rem', + }, + 'size:lg': { + '--spinner-diameter': '2rem', + '--spinner-stroke-width': '0.3125rem', + }, + 'size:xl': { + '--spinner-diameter': 'calc(2rem + 8px)', + '--spinner-stroke-width': '0.375rem', + }, + }, + }, +}); + +// A `Theme` with no parent Theme syncs its name onto the document root so its +// @scope'd component rules also reach portals — which means they reach every +// spinner on the page, including ones rendered outside the provider. A +// "default vs themed" pair inside one story therefore shows two themed rows, +// measured in Chromium; the unthemed reference is the `Sizes` story above. +export const ThemedGeometry: Story = { + name: 'Themed Geometry (per size)', + render: () => ( + + + Themed — rem and calc() diameters; the box tracks the ring + + + + + + + + + + + ), +}; + +// A hairline stroke: geometry themed down to 1px while the diameter stays put. +// Not `0` — one `stroke-width` drives both circles, so a stroke width of `0` is a +// zero-width stroke on each and paints nothing. An arc with no track behind it +// is `--spinner-track-color: transparent`, which is what the subtle shade in +// `ThemedColor` shows. +const themedHairline = defineTheme({ + name: 'spinner-themed-hairline', + components: { + spinner: { + 'size:xl': {'--spinner-stroke-width': '1px'}, + base: {'--spinner-track-color': 'transparent'}, + }, + }, +}); + +// Colors default to each shade's token, so a theme can retune one shade +// without touching the others. `--color-text-blue` over a muted wash reads as +// themed in the monochrome neutral theme, where an accent-muted pair would +// land within a shade of the default. +const themedColor = defineTheme({ + name: 'spinner-themed-color', + components: { + spinner: { + base: { + '--spinner-color': 'var(--color-text-blue)', + '--spinner-track-color': 'var(--color-background-blue)', + }, + 'shade:subtle': {'--spinner-track-color': 'transparent'}, + }, + }, +}); + +export const ThemedColor: Story = { + name: 'Themed Color (per shade)', + render: () => ( + + + Themed — blue arc and wash; the subtle shade drops its track (the + `Shades` story above is the untinted reference — see the note on + `ThemedGeometry` for why it cannot sit in this story) + + + + + + + + + ), +}; + +// One Theme per story, for the same reason: two providers in one story would +// each claim the document root and the last one mounted would paint both rows. +export const ThemedHairlineStroke: Story = { + name: 'Themed Hairline Stroke', + render: () => ( + + + Themed — a 1px hairline stroke over a transparent track + + + + + + ), +}; diff --git a/packages/cli/api/theme/build/build.public-component-vars.test.mjs b/packages/cli/api/theme/build/build.public-component-vars.test.mjs new file mode 100644 index 0000000000000..bfaeff0c5bc7a --- /dev/null +++ b/packages/cli/api/theme/build/build.public-component-vars.test.mjs @@ -0,0 +1,151 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * A component's *documented* theming vars must survive `astryx theme build`. + * + * `validatePrivateVars` rejects a theme that sets a `--_*` var, on the rule + * that private vars are reached through the derived-var pipeline rather than + * written directly. That makes "which prefix a themeable var carries" a + * build-time contract rather than a naming preference — and nothing checked + * the two against each other, so a component could document a var, and ship a + * changeset telling theme authors to set it, that the build then complains + * about (#5214). + * + * The theme this builds is generated FROM each component's own + * `theming.vars[]`, not from a snippet copied into this file. A hand-copied + * snippet only ever proves the builder accepts the string it was handed; a + * doc-driven one fails the moment a component documents a var a theme author + * cannot actually set. It covers every component with public vars, so the + * next one is covered without touching this file. + * + * Asserted on the receipt's `warnings` rather than on a rejection: a private + * var is reported (logged `✗`, collected into the receipt) and the build then + * emits its CSS and resolves anyway. Asserting a throw would pass for the + * wrong reason — it never throws, which is why a throwaway build read as a + * pass on the first version of #5214. + * + * `themeBuild` compiles via @astryxdesign/core's generator, so it needs a built + * core — the `node` project's globalSetup builds it once before workers fork. + */ + +import { + describe, + it, + expect, + beforeAll, + beforeEach, + afterEach, + vi, +} from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import {fileURLToPath} from 'node:url'; +import {themeBuild} from './build.mjs'; +import {loadComponentDoc} from '../../../foundation/discovery/component-loader.mjs'; + +vi.setConfig({testTimeout: 60000}); + +/** Every documented public var, as `{component, key, vars: [{name, value}]}`. */ +const documented = []; + +beforeAll(async () => { + // Core and the CLI ship as siblings, the same resolution build.mjs uses. + const here = path.dirname(fileURLToPath(import.meta.url)); + const coreSrc = path.resolve(here, '../../../../core/src'); + const docs = []; + (function scan(dir) { + for (const entry of fs.readdirSync(dir, {withFileTypes: true})) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name !== 'node_modules' && entry.name !== '__tests__') + scan(full); + } else if (entry.name.endsWith('.doc.mjs')) { + docs.push(full); + } + } + })(coreSrc); + + for (const docPath of docs) { + let doc; + try { + doc = await loadComponentDoc(docPath); + } catch { + continue; + } + const theming = doc?.theming; + // The `defineTheme` key is the first target's class minus the namespace — + // the same derivation `theme targets` and the builder's own validation use. + const key = theming?.targets?.[0]?.className?.replace(/^astryx-/, ''); + // Every var the docs PRESENT as settable — `private: true` is what hides + // one from `astryx component `, so anything without it is something + // a theme author is being told they may write. Deliberately not filtered + // by the `--_` prefix: a var carrying the private prefix while missing the + // private flag is advertised by the CLI and rejected by the builder, and + // that disagreement is the whole thing this test exists to catch. + const publicVars = (theming?.vars || []).filter( + v => typeof v?.name === 'string' && !v.private && !v.derived, + ); + if (!key || publicVars.length === 0) continue; + documented.push({ + component: path.basename(docPath, '.doc.mjs'), + key, + // A length or a color would each need a plausible value; `unset` is + // valid for any custom property and is not what is under test — that a + // theme may NAME the var at all is. + vars: publicVars.map(v => v.name), + }); + } +}); + +let tmpDir; +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-public-vars-')); +}); +afterEach(() => { + fs.rmSync(tmpDir, {recursive: true, force: true}); +}); + +async function buildTheme(name, components) { + const themeFile = path.join(tmpDir, `${name}.mjs`); + fs.writeFileSync( + themeFile, + `export default ${JSON.stringify({name, tokens: {}, components}, null, 2)};\n`, + ); + return themeBuild(`${name}.mjs`, {}, {cwd: tmpDir}); +} + +const privateVarWarnings = result => + (result?.data.warnings ?? []).filter(w => /private var/i.test(w)); + +describe('documented component vars build cleanly', () => { + it('finds components with public theming vars to check', () => { + // A rename that broke the doc scan would otherwise silently empty this + // file out, the way a var-count bail once did in derivedVarRegistry.test. + expect(documented.length).toBeGreaterThan(0); + }); + + it('accepts every var the component docs tell a theme author to set', async () => { + const components = Object.fromEntries( + documented.map(({key, vars}) => [ + key, + {base: Object.fromEntries(vars.map(name => [name, 'unset']))}, + ]), + ); + + const result = await buildTheme('documentedvars', components); + + expect(result).not.toBeNull(); + expect(privateVarWarnings(result)).toEqual([]); + }); + + it('still reports a private var, so the rule this relies on is real', async () => { + // The negative control: if the builder stopped reporting `--_*`, the test + // above would pass for the wrong reason. + const result = await buildTheme('privatevar', { + spinner: {'size:xl': {'--_spinner-diameter': '40px'}}, + }); + + expect(privateVarWarnings(result)).toHaveLength(1); + }); +}); diff --git a/packages/core/src/Spinner/Spinner.doc.mjs b/packages/core/src/Spinner/Spinner.doc.mjs index e538807239214..80baca5232ed9 100644 --- a/packages/core/src/Spinner/Spinner.doc.mjs +++ b/packages/core/src/Spinner/Spinner.doc.mjs @@ -10,8 +10,8 @@ export const docs = { props: [ { name: 'size', - type: "'sm' | 'md' | 'lg'", - description: 'Spinner size (10px, 14px, 18px).', + type: "'sm' | 'md' | 'lg' | 'xl'", + description: 'Spinner size — ring diameter (10px, 14px, 18px, 28px).', default: "'md'", }, { @@ -43,6 +43,12 @@ export const docs = { targets: [ {className: 'astryx-spinner', visualProps: ['size', 'shade']}, ], + vars: [ + {name: '--spinner-diameter', description: "Diameter of the drawn ring. Set it on a size-variant target to retheme what each named size resolves to, e.g. spinner: { 'size:xl': { '--spinner-diameter': '2.5rem' } }. The rendered box is this plus the stroke width on each side, and follows automatically. Any length works — rem, em and calc() are resolved before the ring is drawn.", default: '10px (sm), 14px (md), 18px (lg), 28px (xl)'}, + {name: '--spinner-stroke-width', description: 'Stroke width of both circles the ring is drawn from — the moving arc and the track behind it. Set it per size alongside the diameter. One stroke width drives both, so 0 is honoured as a zero-width stroke and paints nothing at all rather than falling back to the default — for an arc with no track behind it, set --spinner-track-color to transparent instead.', default: '2px (sm), 3px (md), 3px (lg), 4px (xl)'}, + {name: '--spinner-color', description: "Color of the moving arc. Defaults to the shade's token, so set it on a shade-variant target to retheme one shade — spinner: { 'shade:subtle': { '--spinner-color': 'var(--color-text-tertiary)' } } — or on the base target to retheme all four. Accepts any color notation, including var(), color-mix() and currentColor.", default: 'var(--color-accent) (default), var(--color-text-secondary) (subtle), var(--color-on-dark) (onMedia), currentColor (inherit)'}, + {name: '--spinner-track-color', description: 'Color of the track the arc travels on. Set it to `transparent` for an arc with no track. The onMedia and inherit shades draw the track at reduced alpha (30%) so it reads against an arbitrary backdrop; that fade applies to a themed color too.', default: 'var(--color-track) (default, subtle), var(--color-on-dark) (onMedia), currentColor (inherit)'}, + ], }, usage: { description: @@ -63,8 +69,8 @@ export const docsZh = { props: [ { name: 'size', - type: "'sm' | 'md' | 'lg'", - description: '旋转器尺寸（10px、14px、18px）。', + type: "'sm' | 'md' | 'lg' | 'xl'", + description: '旋转器尺寸——环直径（10px、14px、18px、28px）。', default: "'md'", }, { @@ -95,6 +101,12 @@ export const docsZh = { targets: [ {className: 'astryx-spinner', visualProps: ['size', 'shade']}, ], + vars: [ + {name: '--spinner-diameter', description: "绘制环的直径。在尺寸变体目标上设置，以重新定义每个命名尺寸的解析值，例如 spinner: { 'size:xl': { '--spinner-diameter': '2.5rem' } }。渲染盒子的尺寸为该值加上两侧的描边宽度，并自动跟随。支持任意长度单位——rem、em 与 calc() 会在绘制前解析。", default: '10px (sm), 14px (md), 18px (lg), 28px (xl)'}, + {name: '--spinner-stroke-width', description: '绘制环的两个圆——移动圆弧与其后的轨道——的描边宽度。与直径一起按尺寸设置。同一个描边宽度同时驱动两者，因此 0 会被采纳为零宽描边——什么都不绘制，而不会回退到默认值；若想要没有轨道的圆弧，请改将 --spinner-track-color 设为 transparent。', default: '2px (sm), 3px (md), 3px (lg), 4px (xl)'}, + {name: '--spinner-color', description: "运动圆弧的颜色。默认取所在 shade 的令牌，因此可在 shade 变体目标上设置以重新定义单个 shade——spinner: { 'shade:subtle': { '--spinner-color': 'var(--color-text-tertiary)' } }——或在 base 目标上设置以覆盖全部四种。接受任意颜色写法，包括 var()、color-mix() 与 currentColor。", default: 'var(--color-accent)（default）、var(--color-text-secondary)（subtle）、var(--color-on-dark)（onMedia）、currentColor（inherit）'}, + {name: '--spinner-track-color', description: '圆弧所在轨道的颜色。设为 `transparent` 可得到无轨道的圆弧。onMedia 与 inherit 两种 shade 会以降低的透明度（30%）绘制轨道，以便在任意背景上可辨；该淡化同样作用于主题化的颜色。', default: 'var(--color-track)（default、subtle）、var(--color-on-dark)（onMedia）、currentColor（inherit）'}, + ], }, usage: { description: @@ -121,7 +133,7 @@ export const docsDense = { ], }, propDescriptions: { - size: 'Spinner size (10px, 14px, 18px).', + size: 'Spinner size — ring diameter (10px, 14px, 18px, 28px).', shade: 'Color shade for light or dark backgrounds.', label: 'Visible content below spinner. String auto-sets aria-label.', 'aria-label': 'A11y name for screen readers. Defaults to label or "Loading".', diff --git a/packages/core/src/Spinner/Spinner.test.tsx b/packages/core/src/Spinner/Spinner.test.tsx index 52a8b86d01082..518120e3bb1ed 100644 --- a/packages/core/src/Spinner/Spinner.test.tsx +++ b/packages/core/src/Spinner/Spinner.test.tsx @@ -12,6 +12,8 @@ import {describe, it, expect, vi, afterEach} from 'vitest'; import {render, screen} from '@testing-library/react'; import {Spinner} from './Spinner'; +import {defineTheme} from '../theme/defineTheme'; +import {generateThemeCSS} from '../theme/generateThemeRules'; /** sm/md/lg/xl, as Spinner.tsx defines them. */ const SIZES = { @@ -140,6 +142,85 @@ describe('Spinner', () => { const spinner = screen.getByTestId('spinner'); expect(spinner.tagName.toLowerCase()).toBe('span'); }); + + // The box has always been sized by an inline width/height written after the + // caller's `style`, so the component's own size wins over a `style={{width}}` + // passed in. Making the geometry themeable changed what that value is made + // of — a composed var rather than a number — and deliberately not where it is + // written: moving the sizing into a rule would have handed a caller's inline + // width a precedence over the box it has never had. This pins the precedence + // itself, which is the part a consumer could be depending on. + describe('box sizing', () => { + it.each(Object.entries(SIZES))( + 'falls back to the %s frame where no stylesheet has declared the var', + (size, {diameter, border}) => { + render( + , + ); + const box = screen.getByTestId('spinner'); + const expected = `var(--_spinner-box-size, ${diameter + border * 2}px)`; + expect(box.style.width).toBe(expected); + expect(box.style.height).toBe(expected); + }, + ); + + it('keeps its own size over a width the caller passes in style', () => { + render( + , + ); + const box = screen.getByTestId('spinner'); + expect(box.style.width).toBe('var(--_spinner-box-size, 36px)'); + expect(box.style.height).toBe('var(--_spinner-box-size, 36px)'); + // Everything else the caller passed still applies — only the two + // properties the box owns are taken back. + expect(box.style.opacity).toBe('0.5'); + }); + }); + + // The themed geometry resolves in the cascade — jsdom implements no layout + // and no custom-property registration, so no test here can reach what a + // theme actually draws; that is verified in a browser and the numbers are in + // the PR. What IS a contract a unit test can hold is the routing: a theme + // writes an override against the documented key, and it has to come out on + // the selector the component reads from. + describe('a theme reaches the spinner through its public vars', () => { + const cssFor = ( + components: Parameters[0]['components'], + ) => + generateThemeCSS(defineTheme({name: 'spinner-theming', components})) + .component; + + it('scopes a themed size to that size variant', () => { + // Asserting the whole rule, not just the declaration: the same var on + // the bare `.astryx-spinner` would resize every size at once, which is + // the bug a size-variant key exists to avoid. + expect( + cssFor({spinner: {'size:xl': {'--spinner-diameter': '2.5rem'}}}), + ).toContain('.astryx-spinner.xl {\n --spinner-diameter: 2.5rem;'); + }); + + it('scopes a themed color to that shade variant', () => { + expect( + cssFor({ + spinner: {'shade:subtle': {'--spinner-track-color': 'transparent'}}, + }), + ).toContain( + '.astryx-spinner.subtle {\n --spinner-track-color: transparent;', + ); + }); + + it('lets the base target set a value for every size and shade', () => { + expect( + cssFor({spinner: {base: {'--spinner-color': 'var(--color-brand)'}}}), + ).toContain( + '.astryx-spinner {\n --spinner-color: var(--color-brand);', + ); + }); + }); }); describe('Spinner ring', () => { @@ -196,6 +277,28 @@ describe('Spinner ring', () => { ); }); + // The authored dash is the size's own absolute pattern, and stays that way: + // it is what a render with no stylesheet draws, and it is byte-for-byte the + // pattern this component drew before the geometry became themeable. Scaling + // with a themed diameter is the rule's job — it composes the same two + // lengths out of the resolved diameter — and deliberately not `pathLength`'s, + // which rescales against the path length the UA measures on its own + // approximation of the circle and shortens the default arc by 0.64%. + it.each(Object.entries(SIZES))( + 'leaves the %s arc its own absolute dash, with no pathLength', + (size, {diameter}) => { + render( + , + ); + const {track, arc} = circles(); + const [on, off] = dashOf(arc); + expect(on + off).toBeCloseTo(Math.PI * diameter, 6); + expect(arc.getAttribute('pathLength')).toBeNull(); + // The track is a full ring, so it needs neither. + expect(track.getAttribute('pathLength')).toBeNull(); + }, + ); + // The whole point of the SVG ring: the colours come off the cascade, so // nothing has to resolve them in JS. A read reaching the paint path again // fails here. @@ -268,3 +371,64 @@ describe('Spinner ring', () => { } }); }); + +describe('geometry var registration', () => { + /** The shape `registerSpinnerVars` passes; jsdom implements no real one. */ + type Descriptor = { + name: string; + syntax: string; + inherits: boolean; + initialValue: string; + }; + const stubRegisterProperty = (fn: (d: Descriptor) => void) => { + ( + CSS as unknown as {registerProperty: (d: Descriptor) => void} + ).registerProperty = fn; + }; + + afterEach(() => { + delete (CSS as unknown as {registerProperty?: unknown}).registerProperty; + vi.resetModules(); + }); + + it('registers when the module is imported, not when a spinner mounts', async () => { + // Registering an inherited property with an `initial-value` invalidates + // style for the whole document. A spinner mounts onto a page that has + // already rendered, so paying it there is paying it on the full tree — + // measured at 29ms against 12ms on an 11k-element page. At import the page + // is whatever has rendered so far, which for a bundle in the head is + // nothing. This pins the timing: it fails if the call moves back into a + // ref callback, an effect, or the component body. + const registerProperty = vi.fn<(d: Descriptor) => void>(); + stubRegisterProperty(registerProperty); + + vi.resetModules(); + await import('./Spinner'); + + expect(registerProperty.mock.calls.map(([d]) => d.name)).toEqual([ + '--_spinner-ring-diameter', + '--_spinner-ring-stroke', + ]); + // Both are `` with an initial value, which is what makes a themed + // `0` mean `0px` inside the `calc()` rather than poisoning it. + for (const [descriptor] of registerProperty.mock.calls) { + expect(descriptor.syntax).toBe(''); + expect(descriptor.inherits).toBe(true); + expect(descriptor.initialValue).toBe('0px'); + } + }); + + it('survives a second evaluation, and does not need the DOM', async () => { + // Two copies of the package on one page, or a fast-refresh re-evaluation: + // registerProperty throws on a duplicate rather than replacing, and the + // existing registration is this same one. + const registerProperty = vi.fn<(d: Descriptor) => void>(() => { + throw new Error('InvalidModificationError'); + }); + stubRegisterProperty(registerProperty); + + vi.resetModules(); + await expect(import('./Spinner')).resolves.toBeDefined(); + expect(registerProperty).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/core/src/Spinner/Spinner.tsx b/packages/core/src/Spinner/Spinner.tsx index 2809df2b10cf6..04ef97535215b 100644 --- a/packages/core/src/Spinner/Spinner.tsx +++ b/packages/core/src/Spinner/Spinner.tsx @@ -34,6 +34,16 @@ import {themeProps} from '../utils/themeProps'; */ const ARC_FRACTION = 0.375; +/** + * The dash pattern, per unit of diameter: one arc, then the gap that closes + * the circle. The circumference is `pi x diameter`, so multiplying the + * resolved diameter by these two constants gives exactly the lengths the + * default render has always used, and scales them with a themed diameter. + */ +const PI = 3.141592653589793; +const ARC_DASH = PI * ARC_FRACTION; +const ARC_GAP = PI * (1 - ARC_FRACTION); + const SIZES = { sm: {diameter: 10, border: 2}, md: {diameter: 14, border: 3}, @@ -41,14 +51,103 @@ const SIZES = { xl: {diameter: 28, border: 4}, }; -/** `onMedia` keeps the 77/255 its token's `4D` hex suffix used to encode. */ -const TRACK_OPACITY = { +/** + * Opacity the track is drawn at, per shade. `77 / 255` is the `4D` the onMedia + * track used to append to the token's hex — the same composite, but applied as + * `stroke-opacity` to a color off the cascade, so it no longer depends on the + * token being hex notation and it applies to a themed color too. + */ +const TRACK_OPACITY: Record = { default: 1, subtle: 1, onMedia: 77 / 255, inherit: 0.3, }; +/** + * Where the resolved geometry lands: the public var, resolved into a registered + * `` the `calc()`s below can do arithmetic on. The box and the ring + * read these; the public vars are declared once, on the element carrying the + * theme target, by `sizeStyles`. + */ +const RESOLVED_DIAMETER = '--_spinner-ring-diameter'; +const RESOLVED_STROKE = '--_spinner-ring-stroke'; +const RESOLVED_GEOMETRY_VARS = [RESOLVED_DIAMETER, RESOLVED_STROKE]; + +/** + * The composed box size: diameter plus a stroke width on each side. + * + * It is deliberately NOT registered, unlike the pair above. The element reads + * it through an inline `width`/`height` with the size's own default as the + * `var()` fallback, so a render with no stylesheet — where nothing declares + * it — still sizes the box the way it always did. A registered property always + * has a value (its `initial-value`), which would swallow that fallback and + * collapse the box to zero. + */ +const BOX_SIZE = '--_spinner-box-size'; + +/** + * Register the resolved geometry vars as ``. + * + * Both are consumed inside `calc()` — the box adds two stroke widths to a diameter, + * the circle halves one. Unregistered, a custom property substitutes as text, + * so whatever a theme wrote lands in the expression verbatim and a bare `0` + * (a valid length on its own, a `` inside `calc()`) poisons the sum: + * `calc(28px + 0 * 2)` is invalid at computed-value time and the box loses its + * size. Registered, the value is already an absolute length by the time the + * `calc()` sees it, so `0` means `0px` — a zero-width stroke that paints + * nothing — rather than a bare `0` that invalidates the sum and leaves the box + * with no size at all. One `stroke-width` drives both circles, so a themed + * stroke width of `0` hides the arc along with the track; an arc with no track behind + * it is `--spinner-track-color: transparent`. + * + * Only these private vars are registered. The four public ones deliberately + * are not: a registered property has an `initial-value`, so every element in + * the document would report a value for it — and + * `.github/scripts/theme-var-reachability.js` finds a var's declaring element + * by exactly that test, so registering them would point the guard at `` + * and report a var no theme can select. + */ +function registerSpinnerVars(): void { + if ( + typeof CSS === 'undefined' || + typeof CSS.registerProperty !== 'function' + ) { + return; + } + for (const name of RESOLVED_GEOMETRY_VARS) { + try { + CSS.registerProperty({ + name, + syntax: '', + inherits: true, + initialValue: '0px', + }); + } catch { + // Already registered — a second copy of the package on the page, or a + // fast-refresh re-evaluation. registerProperty throws rather than + // replacing, and the existing registration is this same one. + } + } +} + +// Registering an inherited property with an `initial-value` invalidates style +// for the whole document, so this runs when the module is evaluated rather +// than when a spinner mounts. A spinner is the loading indicator: it mounts +// onto a page that is already rendered, with someone already waiting, and the +// recalc it triggers there is paid on the full tree. At import the tree is +// whatever has rendered so far, which for a bundle loaded in the head is +// nothing. +// +// It is safe at module scope in both directions. The `typeof CSS` guard above +// keeps it out of the server render, and tree-shaking cannot strip it from a +// build that renders a spinner: core's `sideEffects` allowlist does not name +// this file, so a bundle that never imports `Spinner` drops the module whole — +// registration and all, which is the outcome you want — while one that does +// import it keeps the module, and a bare call is not something a bundler may +// elide. +registerSpinnerVars(); + /** * Pin every ring's rotation to the document timeline's origin instead of its * own start time, so spinners mounted seconds apart turn in phase. @@ -78,12 +177,20 @@ function pinRingsToTimelineOrigin(): void { } } +/** + * Ref callback for the ring: the one place a mounted ring touches the DOM. + * + * It reads nothing back — the geometry is resolved by the cascade, not in JS. + */ function syncRotationPhase( svg: SVGSVGElement | null, ): (() => void) | undefined { + if (svg == null) { + return undefined; + } // jsdom implements no Web Animations, and this runs in every consumer's // component tests. - if (svg == null || typeof svg.getAnimations !== 'function') { + if (typeof svg.getAnimations !== 'function') { return undefined; } pendingRings.add(svg); @@ -121,11 +228,31 @@ const styles = stylex.create({ placeItems: 'center', overflow: 'hidden', verticalAlign: 'middle', + // The public geometry vars, resolved into the registered `` pair + // the arithmetic below needs. Reading them here rather than in each + // `calc()` keeps one place where a themed value enters the component, and + // it is the span that reads them whether the theme target is the span or + // the wrapper — a custom property inherits either way. + [RESOLVED_DIAMETER]: 'var(--spinner-diameter)', + [RESOLVED_STROKE]: 'var(--spinner-stroke-width)', + // The size of the box, composed here and applied as an inline style at the + // element, so that the box and the drawn ring come from the same two vars + // and a themed size moves both together — without the sizing moving from + // an inline style to a rule, which would hand a caller's `style={{width}}` + // a precedence over the box that it has never had. + [BOX_SIZE]: `calc(var(${RESOLVED_DIAMETER}) + var(${RESOLVED_STROKE}) * 2)`, }, ring: { backfaceVisibility: 'hidden', display: 'block', willChange: 'transform', + // The svg keeps the size its `viewBox` describes, so one user unit is one + // CSS pixel and the lengths below mean what they say. A themed diameter + // therefore draws a ring wider than the svg's own box — which is fine, and + // stays centered, because the box it is centered in is the span, sized + // from the same two vars. Clipping it to the default frame is the one + // thing that would break that, hence `visible`. + overflow: 'visible', // Slow the rotation dramatically under reduced-motion rather than freezing // it (a frozen spinner reads as broken), matching ProgressBar's approach. // The role="status" + "Loading" label still convey busy state (obs-6). @@ -140,30 +267,98 @@ const styles = stylex.create({ circle: { fill: 'none', strokeLinecap: 'round', + // The geometry the ring is actually drawn at. `r` and `stroke-width` are + // CSS properties on an SVG shape, and a CSS declaration outranks the + // presentation attribute of the same name — so the attributes below stay + // as the size's default (and as what a server render and a no-CSS render + // draw), and these take over the moment the cascade has a themed value. + r: `calc(var(${RESOLVED_DIAMETER}) / 2)`, + strokeWidth: `var(${RESOLVED_STROKE})`, }, + // The two ring colors ride `stroke` directly, read off the public vars the + // shade declares. The paint comes from the cascade, so every notation a + // theme can write — `var()`, `color-mix()`, and the `currentColor` the + // inherit shade is built on — resolves where it is used, and a color changed + // after mount repaints instead of going stale. + // + // The dash pattern is composed from the resolved diameter the same way, so a + // themed ring keeps the same fraction of arc rather than the same absolute + // dash. `pathLength` would be the shorter route to that, but it rescales the + // pattern against the path length the UA measures on its own approximation + // of the circle — 87.398 against the 87.965 of pi x 28 — which shortens the + // default arc by 0.64% and moves the cap by half a pixel. Composing the + // lengths keeps the default byte-identical to what it drew before. + arc: { + stroke: 'var(--spinner-color)', + strokeDasharray: `calc(var(${RESOLVED_DIAMETER}) * ${ARC_DASH}) calc(var(${RESOLVED_DIAMETER}) * ${ARC_GAP})`, + }, + track: {stroke: 'var(--spinner-track-color)'}, }); -const arcStyles = stylex.create({ - default: {stroke: colorVars['--color-accent']}, - subtle: {stroke: colorVars['--color-text-secondary']}, - onMedia: {stroke: colorVars['--color-on-dark']}, - inherit: {stroke: 'currentColor'}, +// What each named `size` and `shade` resolve to. Both groups DECLARE the four +// public vars, on the element that carries the `spinner` theme target, and +// everything downstream reads them — so a theme's `@layer astryx-theme` rule +// against `.astryx-spinner.xl` overrides the default the same way it does for +// `--tree-list-indent` or `--button-focus-offset`, e.g. +// spinner: { 'size:xl': { '--spinner-diameter': '40px' } }. +// +// Declaring is only safe because #5410 moved the compiled StyleX CSS inside +// `@layer astryx-base`. Before it, StyleX emitted custom-property +// declarations at priority 0 and therefore OUTSIDE its layers, and an +// unlayered declaration beats every layer — so a StyleX-declared +// `--spinner-diameter: 10px` shadowed the theme's own rule no matter how +// specific the theme got. An earlier revision of this component worked around +// that by never declaring the public var and reading it with the default as a +// `var()` fallback; that is no longer necessary, and the fallback shape has a +// cost of its own — with nothing declaring the var, +// `theme-var-reachability.js` cannot find an element to check, so a documented +// var reads as unreachable. +const sizeStyles = stylex.create({ + sm: { + '--spinner-diameter': `${SIZES.sm.diameter}px`, + '--spinner-stroke-width': `${SIZES.sm.border}px`, + }, + md: { + '--spinner-diameter': `${SIZES.md.diameter}px`, + '--spinner-stroke-width': `${SIZES.md.border}px`, + }, + lg: { + '--spinner-diameter': `${SIZES.lg.diameter}px`, + '--spinner-stroke-width': `${SIZES.lg.border}px`, + }, + xl: { + '--spinner-diameter': `${SIZES.xl.diameter}px`, + '--spinner-stroke-width': `${SIZES.xl.border}px`, + }, }); -const trackStyles = stylex.create({ +const shadeStyles = stylex.create({ default: { - stroke: colorVars['--color-track'], - strokeOpacity: TRACK_OPACITY.default, + '--spinner-color': colorVars['--color-accent'], + '--spinner-track-color': colorVars['--color-track'], }, subtle: { - stroke: colorVars['--color-track'], - strokeOpacity: TRACK_OPACITY.subtle, + '--spinner-color': colorVars['--color-text-secondary'], + '--spinner-track-color': colorVars['--color-track'], }, onMedia: { - stroke: colorVars['--color-on-dark'], - strokeOpacity: TRACK_OPACITY.onMedia, + '--spinner-color': colorVars['--color-on-dark'], + '--spinner-track-color': colorVars['--color-on-dark'], + }, + inherit: { + '--spinner-color': 'currentColor', + '--spinner-track-color': 'currentColor', }, - inherit: {stroke: 'currentColor', strokeOpacity: TRACK_OPACITY.inherit}, +}); + +// The track's alpha is a property, not a color: it composites over whatever +// color the shade or the theme supplies. `77 / 255` is the `4D` the onMedia +// token's hex used to carry. +const trackOpacityStyles = stylex.create({ + default: {strokeOpacity: TRACK_OPACITY.default}, + subtle: {strokeOpacity: TRACK_OPACITY.subtle}, + onMedia: {strokeOpacity: TRACK_OPACITY.onMedia}, + inherit: {strokeOpacity: TRACK_OPACITY.inherit}, }); // ============================================================================= @@ -178,7 +373,9 @@ export interface SpinnerProps extends BaseProps { /** Ref forwarded to the root element */ ref?: React.Ref; /** - * Spinner size. + * Spinner size. The diameter is the ring itself; the rendered box adds the + * stroke width on each side (xl draws a 28px ring in a 36px box). A theme can + * redefine what each named size resolves to — see `--spinner-diameter`. * - 'sm': 10px diameter * - 'md': 14px diameter * - 'lg': 18px diameter @@ -275,9 +472,30 @@ export function Spinner({ {...(hasLabel ? {} : restProps)} {...mergeProps( hasLabel ? '' : themeProps('spinner', {size, shade}), - stylex.props(styles.spinner, !hasLabel && xstyle), + stylex.props( + styles.spinner, + // The defaults are declared on whichever element carries the theme + // target, and only there: when a label moves the target to the + // wrapper, this span must inherit the wrapper's value rather than + // declare its own, which would shadow a theme's override with the + // default it is trying to replace. + !hasLabel && sizeStyles[size], + !hasLabel && shadeStyles[shade], + !hasLabel && xstyle, + ), hasLabel ? undefined : className, - {...(hasLabel ? {} : style), width: frameSize, height: frameSize}, + // The box is sized here, after the caller's `style`, exactly as it was + // before the geometry became themeable: the component's own size wins + // over a `style={{width}}` a caller passes, and the precedence between + // the two is unchanged by this PR. What the value is made of has + // changed — it is the composed var rather than a number — so a themed + // diameter moves the box with the ring. The fallback is the size's own + // frame, for the render where no stylesheet has declared the var. + { + ...(hasLabel ? {} : style), + width: `var(${BOX_SIZE}, ${frameSize}px)`, + height: `var(${BOX_SIZE}, ${frameSize}px)`, + }, )}> @@ -317,7 +542,12 @@ export function Spinner({ {...restProps} {...mergeProps( themeProps('spinner', {size, shade}), - stylex.props(styles.wrapper, xstyle), + stylex.props( + styles.wrapper, + sizeStyles[size], + shadeStyles[shade], + xstyle, + ), className, style, )}> diff --git a/packages/core/src/theme/derivedVarRegistry.test.ts b/packages/core/src/theme/derivedVarRegistry.test.ts index 1bbe9681a2de8..8aca3c713d550 100644 --- a/packages/core/src/theme/derivedVarRegistry.test.ts +++ b/packages/core/src/theme/derivedVarRegistry.test.ts @@ -300,6 +300,15 @@ const VARS_WITHOUT_DERIVED_MAPPING = new Set([ // It is one component of one shadow in the list, so no standard property // maps onto it either — a theme sets it beside the fill it has to contrast. '--selectable-card-ring-color', + // The spinner's ring is drawn as an SVG circle, so none of its four vars is + // a CSS property of the element carrying the theme target: `width` and + // `borderWidth` would name a box the ring is not, and a `color` mapping + // would take the label's text color with it. They are public vars a theme + // sets directly under a size- or shade-variant key. + '--spinner-diameter', + '--spinner-stroke-width', + '--spinner-color', + '--spinner-track-color', ]); // ---------------------------------------------------------------------------