Skip to content

feat(theme): make color.accent optional in defineTheme - #3916

Merged
cixzhang merged 1 commit into
facebook:mainfrom
AKnassa:feat/2279-optional-accent
Jul 31, 2026
Merged

feat(theme): make color.accent optional in defineTheme#3916
cixzhang merged 1 commit into
facebook:mainfrom
AKnassa:feat/2279-optional-accent

Conversation

@AKnassa

@AKnassa AKnassa commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

What this does

You can now build a theme that only restyles the greys — without having to pick a brand colour you don't want.

Why

defineTheme insisted on an accent colour. So if all you wanted was warmer greys, or higher contrast, you still had to supply an accent — and whatever you picked would silently become your theme's brand colour.

This is the maintainer's own request in #2279. It implements only suggested fix #1 from that issue.

What changed

  • color.accent is now optional.
  • Leave it out and you still get the full grey ramp (neutralStyle, contrast all work) — the greys borrow their hue from the default accent.
  • The three accent tokens (--color-accent, --color-accent-muted, --color-on-accent) are simply not generated, so they fall back to the default values. This is the same fall-through the file already uses for status, categorical and on-dark tokens.
  • If you do pass an accent, nothing changes — output is identical, token for token.
// now valid — warm greys, default accent left alone
defineTheme({name: 'warm', color: {neutralStyle: 'warm'}})

One thing worth a close look 👀

The obvious-looking implementation would be "if there's no accent, just default it to the standard accent hex." That is wrong, and it would quietly re-colour every existing theme.

The accent colour you get by deriving a scale from #0064E0 is not the same value as the default accent token:

value
expandColorScale({accent: '#0064E0'})['--color-accent'] light-dark(#0058D2, #BBC2FF)
colorDefaults['--color-accent'] light-dark(#0064E0, #2694FE)

So the accent tokens are omitted, not defaulted — omitting them is what preserves the existing default. There's a test pinning both values, so if anyone later "simplifies" this into the seeded version, it fails loudly instead of silently changing everyone's brand colour.

Scope

Only suggested fix #1. Deliberately not implementing #2 (tuple/per-scheme accents) or #3 — those touch accent-picking semantics and belong with the in-flight Color Studio work.

Checks

  • Core + CLI suites green; lint, typecheck, prettier clean
  • Tests cover: neutral-only themes end-to-end through useTheme() in light and dark; an empty config; an empty-string accent (found and fixed a real bug here — the seed used a nullish check while the token output used a truthy one, so accent: '' seeded from a malformed value and dropped the accent tokens); a neutral-only theme with a hand-picked accent override; and a drift guard tying the seed to the default token

ColorScaleConfig required an accent, so a theme could not restyle only the
neutral ramp -- defineTheme({color: {neutralStyle: 'warm'}}) was a type error.

accent is now optional. Without one, the neutral palettes seed from the default
accent's hue and --color-accent, --color-accent-muted and --color-on-accent are
left ungenerated so they fall through to colorDefaults -- the same fall-through
expandColorScale already applies to status, categorical and on-dark tokens.

Note the accent tokens are omitted, NOT defaulted: the scale derived from the
default accent hex is not the same value as the default accent token
(expandColorScale({accent:'#0064E0'}) yields light-dark(#0058D2, #BBC2FF) vs
colorDefaults light-dark(#0064E0, #2694FE)), so seeding the default would
silently re-color every existing theme. A test pins both against drift.

Configs that pass an accent are unchanged, token for token.
@vercel

vercel Bot commented Jul 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 13, 2026 9:14pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 13, 2026
@github-actions github-actions Bot added community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge labels Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Analysis Report

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

@AKnassa
AKnassa marked this pull request as ready for review July 14, 2026 02:30

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really clean fill for the accent-required gap in #2279 (and the friction called out in the adoption feedback). Making color.accent optional so a theme can restyle just the neutral ramp — neutralStyle, contrast — without being forced to adopt a brand color is exactly the ergonomic win the issue asked for.

What makes this mergeable rather than merely plausible is that you got the backward-compatibility question exactly right: when accent is omitted, the three accent tokens are omitted, not seeded — so they fall through to colorDefaults (the same fall-through the file already uses for status/categorical/on-dark tokens), and every theme that does pass an accent stays byte-identical, token for token. The subtle-but-critical detail — that deriving a scale from #0064E0 produces a different accent than the default token holds, so "just default the seed" would silently re-color existing themes — is called out and pinned by a drift-guard test. The suite covers omit-vs-seed, empty config, the accent: '' nullish-vs-truthy edge, runtime resolution through useTheme() in both modes, and an explicit-accent override. Nicely scoped, too: it deliberately stays out of the tuple/per-scheme accent and inversion territory.

Verified locally on a fresh worktree: the expandColorScale suite (17 tests) and tsc both pass, and the accent-present branch is unchanged from main. Thanks for the thorough tests and the clear writeup.

@github-actions github-actions Bot removed the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Jul 31, 2026
@cixzhang
cixzhang merged commit 0fe271d into facebook:main Jul 31, 2026
23 checks passed
@github-actions
github-actions Bot deleted the feat/2279-optional-accent branch July 31, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants