Skip to content

fix(playground): prevent derived color scale from overriding custom accent color#3269

Open
GeekLuffy wants to merge 2 commits into
facebook:mainfrom
GeekLuffy:fix/theme-creator-accent-picker
Open

fix(playground): prevent derived color scale from overriding custom accent color#3269
GeekLuffy wants to merge 2 commits into
facebook:mainfrom
GeekLuffy:fix/theme-creator-accent-picker

Conversation

@GeekLuffy

Copy link
Copy Markdown

Summary

This PR fixes a bug in the playground theme editor where toggling "Create from accent" and selecting/typing an accent color causes the input to get "stuck" or automatically shift to a different, derived color (the tone-40/tone-80 HCT tone variant).

Cause

When "Create from accent" is active, changing --color-accent calls onExpandColorScale(hex) to derive all semantic scale tokens. However, the derived scale includes a generated --color-accent token value (light-dark(P[40], P[80])). When merged back into the React state batch, this derived value overrides the exact custom accent color the user selected or typed in the color swatch.

Fix

Deleted --color-accent from the derived token object in handleExpandColorScale (inside ThemeEditor.tsx) before merging it into the token state. This preserves the user's custom accent color selection while successfully generating all other neutral and semantic color tokens from it.

Verification

  • Manual Check: Verified in the playground with "Create from accent" active that entering any custom hex values (like #ff0000 100%) correctly saves and updates without jumping/reverting.
  • Build Check: Ran npx pnpm build in the docsite to verify compilation is successful with no errors.

Closes #3238

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 30, 2026
@vercel

vercel Bot commented Jun 30, 2026

Copy link
Copy Markdown

@GeekLuffy is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@GeekLuffy GeekLuffy force-pushed the fix/theme-creator-accent-picker branch from 72b1158 to e79e8e3 Compare June 30, 2026 05:32
@ernestt

ernestt commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Heads up on the checks here — they're not failing or hung on your end. Because this is a PR from a fork by a first-time contributor, GitHub holds the workflow runs (CI, Lint, Internal Registry) in the action_required state until a maintainer approves them. That's why they look stuck with no results, and why the Vercel preview shows "Authorization required to deploy" rather than an actual build error. Your CLA check has already passed.

A maintainer just needs to hit Approve and run workflows on the Checks tab and they'll execute normally — nothing to change in the code for that.

On the fix itself: the diagnosis matches the code path. With "Create from accent" on, an accent edit batches two state updates — the swatch's own onTokenChange('--color-accent', ...) and then onExpandColorScale, whose derived scale re-emits --color-accent as the tone-mapped light-dark(P[40], P[80]) and clobbers the raw pick. Deleting --color-accent from derived in handleExpandColorScale is the right place to fix it: both trigger paths (the toggle and the swatch) funnel through that one handler, and it leaves the core expandColorScale util untouched for defineTheme. One optional suggestion — a small unit test asserting handleExpandColorScale output omits --color-accent would lock this in against regressions.

@GeekLuffy

Copy link
Copy Markdown
Author

Heads up on the checks here — they're not failing or hung on your end. Because this is a PR from a fork by a first-time contributor, GitHub holds the workflow runs (CI, Lint, Internal Registry) in the action_required state until a maintainer approves them. That's why they look stuck with no results, and why the Vercel preview shows "Authorization required to deploy" rather than an actual build error. Your CLA check has already passed.

A maintainer just needs to hit Approve and run workflows on the Checks tab and they'll execute normally — nothing to change in the code for that.

On the fix itself: the diagnosis matches the code path. With "Create from accent" on, an accent edit batches two state updates — the swatch's own onTokenChange('--color-accent', ...) and then onExpandColorScale, whose derived scale re-emits --color-accent as the tone-mapped light-dark(P[40], P[80]) and clobbers the raw pick. Deleting --color-accent from derived in handleExpandColorScale is the right place to fix it: both trigger paths (the toggle and the swatch) funnel through that one handler, and it leaves the core expandColorScale util untouched for defineTheme. One optional suggestion — a small unit test asserting handleExpandColorScale output omits --color-accent would lock this in against regressions.

Thanks for the detailed review!

I have implemented your suggestion:

  1. Extracted the expansion logic from handleExpandColorScale into a testable pure function getExpandedColorScale in helpers.ts.
  2. Refactored ThemeEditor.tsx to use this new function.
  3. Added unit tests in apps/docsite/src/__tests__/theme-helpers.test.ts to assert that getExpandedColorScale correctly computes all derived tokens but excludes --color-accent.

I've pushed the updated commits to the PR branch. Ready for review/approval!

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Theme playground "Create from accent" doesn't work

2 participants