feat(theme): scoped theming with ThemeScope component#768
feat(theme): scoped theming with ThemeScope component#768paanSinghCoder wants to merge 3 commits into
Conversation
Any element with `data-theme` (or wrapped in `<ThemeScope>`) now creates an isolated theme region. Descendants resolve all `--rs-color-*` tokens, `color-scheme`, and the smooth theme-switch transition from the nearest scoped ancestor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces scoped theming for the Raystack design system. It enables nested Sequence DiagramsequenceDiagram
participant App as Page Component
participant Theme as ThemeProvider (outer)
participant Scoped as Scoped Wrapper (inner)
participant DOM as DOM Elements
App->>Theme: useTheme() for global theme
Theme->>DOM: writes data-theme to document root
App->>Theme: render nested ThemeProvider
Theme->>Scoped: detects parent context, render Scoped
Scoped->>DOM: writes data-theme to wrapper div
Scoped->>DOM: writes data-accent-color if accentColor prop
Scoped->>DOM: writes data-gray-color if grayColor prop
Scoped->>DOM: writes data-style if style prop
DOM->>DOM: CSS [data-theme="light/dark"] selectors resolve tokens
DOM->>App: child components render with scoped tokens
Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/app/examples/scoped-theme/page.tsx`:
- Around line 17-23: The panelStyle object uses a spacing token for borderRadius
(borderRadius: 'var(--rs-space-3)') which teaches the wrong token family; update
the borderRadius value to use the appropriate radius token (e.g., replace
var(--rs-space-3) with the project's radius token such as var(--rs-radius-3) or
the correct --rs-radius-<n>), keeping the rest of panelStyle unchanged so the
example demonstrates the proper radius token usage.
In `@packages/raystack/styles/primitives/appearance.css`:
- Around line 13-20: The CSS unconditionally applies color-scheme when
[data-theme] is present, bypassing ThemeProvider's enableColorScheme flag;
update the rules to only apply when an explicit opt-in marker is present (e.g.,
[data-enable-color-scheme="true"] or a class like .enable-color-scheme) so
ThemeProvider's enableColorScheme prop in ThemeProvider
(packages/raystack/components/theme-provider/theme.tsx) can control whether
native color-scheme is set; ensure the marker is added/removed by ThemeProvider
where enableColorScheme is read so disabling the prop actually prevents the CSS
color-scheme rules from applying.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68a8df63-d9e9-4b91-beba-bc0d1b81312d
📒 Files selected for processing (13)
apps/www/src/app/examples/scoped-theme/page.tsxapps/www/src/content/docs/theme/overview/index.mdxapps/www/src/content/docs/theme/overview/props.tspackages/raystack/components/theme-provider/__tests__/theme-scope.test.tsxpackages/raystack/components/theme-provider/index.tsxpackages/raystack/components/theme-provider/theme-scope.tsxpackages/raystack/components/theme-provider/theme.tsxpackages/raystack/components/theme-provider/types.tspackages/raystack/index.tsxpackages/raystack/styles/colors.csspackages/raystack/styles/primitives/accent.csspackages/raystack/styles/primitives/appearance.csspackages/raystack/styles/primitives/gray.css
| const panelStyle = { | ||
| minWidth: 320, | ||
| padding: 'var(--rs-space-7)', | ||
| borderRadius: 'var(--rs-space-3)', | ||
| border: '1px solid var(--rs-color-border-base-primary)', | ||
| backgroundColor: 'var(--rs-color-background-base-primary)' | ||
| }; |
There was a problem hiding this comment.
Use a radius token for borderRadius here.
borderRadius is pulling from the spacing scale. That makes the example teach the wrong token family and can drift if spacing and radius scales stop lining up.
Suggested fix
const panelStyle = {
minWidth: 320,
padding: 'var(--rs-space-7)',
- borderRadius: 'var(--rs-space-3)',
+ borderRadius: 'var(--rs-radius-3)',
border: '1px solid var(--rs-color-border-base-primary)',
backgroundColor: 'var(--rs-color-background-base-primary)'
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const panelStyle = { | |
| minWidth: 320, | |
| padding: 'var(--rs-space-7)', | |
| borderRadius: 'var(--rs-space-3)', | |
| border: '1px solid var(--rs-color-border-base-primary)', | |
| backgroundColor: 'var(--rs-color-background-base-primary)' | |
| }; | |
| const panelStyle = { | |
| minWidth: 320, | |
| padding: 'var(--rs-space-7)', | |
| borderRadius: 'var(--rs-radius-3)', | |
| border: '1px solid var(--rs-color-border-base-primary)', | |
| backgroundColor: 'var(--rs-color-background-base-primary)' | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/www/src/app/examples/scoped-theme/page.tsx` around lines 17 - 23, The
panelStyle object uses a spacing token for borderRadius (borderRadius:
'var(--rs-space-3)') which teaches the wrong token family; update the
borderRadius value to use the appropriate radius token (e.g., replace
var(--rs-space-3) with the project's radius token such as var(--rs-radius-3) or
the correct --rs-radius-<n>), keeping the rest of panelStyle unchanged so the
example demonstrates the proper radius token usage.
| /* Native UI (form controls, scrollbars, text selection) follows the scoped theme */ | ||
| [data-theme="light"] { | ||
| color-scheme: light; | ||
| } | ||
|
|
||
| [data-theme="dark"] { | ||
| color-scheme: dark; | ||
| } |
There was a problem hiding this comment.
enableColorScheme={false} is bypassed by unconditional scoped color-scheme.
ThemeProvider still models enableColorScheme as a conditional behavior (packages/raystack/components/theme-provider/theme.tsx, Line 40 and Lines 90-99 / 258-271), but this CSS now forces native color-scheme whenever data-theme is present. That makes the prop effectively non-functional.
Please gate these rules behind an explicit opt-in attribute/class so disabling enableColorScheme is still respected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/styles/primitives/appearance.css` around lines 13 - 20, The
CSS unconditionally applies color-scheme when [data-theme] is present, bypassing
ThemeProvider's enableColorScheme flag; update the rules to only apply when an
explicit opt-in marker is present (e.g., [data-enable-color-scheme="true"] or a
class like .enable-color-scheme) so ThemeProvider's enableColorScheme prop in
ThemeProvider (packages/raystack/components/theme-provider/theme.tsx) can
control whether native color-scheme is set; ensure the marker is added/removed
by ThemeProvider where enableColorScheme is read so disabling the prop actually
prevents the CSS color-scheme rules from applying.
|
@rohilsurana Closing this PR for now. We will pick this later when we have defined usecase and better scoping. |
ThemeProvider auto-detects scope vs root via context: nested usage renders a wrapper `<div>` with `data-*` overrides instead of a no-op fragment. Drops the ThemeScope component (and its render-prop/Base UI dep) so consumers have one entry point. useTheme() inside a scope still returns the root provider's state — scoped mode is stateless by design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/raystack/components/theme-provider/__tests__/theme-scope.test.tsx (1)
6-27: ⚡ Quick winIsolate and restore global
windowmocks to avoid cross-suite leakage.Lines 6–27 define
localStorageandmatchMediaat module scope without restoration, allowing mocks to leak across test suites and create order-dependent failures. Move setup intobeforeAlland restore original property descriptors inafterAll.Proposed refactor
-import { describe, expect, it, vi } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { ThemeProvider } from '../theme'; -// jsdom doesn't ship these; the root ThemeProvider needs them on mount. -Object.defineProperty(window, 'localStorage', { - value: { - getItem: vi.fn(), - setItem: vi.fn(), - removeItem: vi.fn(), - clear: vi.fn() - } -}); - -Object.defineProperty(window, 'matchMedia', { - writable: true, - value: vi.fn().mockImplementation(query => ({ - matches: false, - media: query, - onchange: null, - addListener: vi.fn(), - removeListener: vi.fn(), - addEventListener: vi.fn(), - removeEventListener: vi.fn(), - dispatchEvent: vi.fn() - })) -}); +const originalLocalStorage = Object.getOwnPropertyDescriptor(window, 'localStorage'); +const originalMatchMedia = Object.getOwnPropertyDescriptor(window, 'matchMedia'); + +beforeAll(() => { + Object.defineProperty(window, 'localStorage', { + configurable: true, + value: { + getItem: vi.fn(), + setItem: vi.fn(), + removeItem: vi.fn(), + clear: vi.fn() + } + }); + + Object.defineProperty(window, 'matchMedia', { + configurable: true, + writable: true, + value: vi.fn().mockImplementation(query => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn() + })) + }); +}); + +afterAll(() => { + if (originalLocalStorage) { + Object.defineProperty(window, 'localStorage', originalLocalStorage); + } + if (originalMatchMedia) { + Object.defineProperty(window, 'matchMedia', originalMatchMedia); + } +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/theme-provider/__tests__/theme-scope.test.tsx` around lines 6 - 27, Move the global window mocks for localStorage and matchMedia out of module scope and into a test lifecycle so they are restored after the suite: capture the original descriptors for window.localStorage and window.matchMedia, set the mocked implementations inside beforeAll (using the current mock implementations from the diff), and restore the originals in afterAll; reference the mocked symbols window.localStorage and window.matchMedia and the lifecycle hooks beforeAll/afterAll to locate where to apply the changes so other suites are not affected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/raystack/components/theme-provider/__tests__/theme-scope.test.tsx`:
- Around line 6-27: Move the global window mocks for localStorage and matchMedia
out of module scope and into a test lifecycle so they are restored after the
suite: capture the original descriptors for window.localStorage and
window.matchMedia, set the mocked implementations inside beforeAll (using the
current mock implementations from the diff), and restore the originals in
afterAll; reference the mocked symbols window.localStorage and window.matchMedia
and the lifecycle hooks beforeAll/afterAll to locate where to apply the changes
so other suites are not affected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38904419-ed26-4f20-9c55-f34a82462c53
📒 Files selected for processing (4)
apps/www/src/app/examples/scoped-theme/page.tsxapps/www/src/content/docs/theme/overview/index.mdxpackages/raystack/components/theme-provider/__tests__/theme-scope.test.tsxpackages/raystack/components/theme-provider/theme.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/www/src/content/docs/theme/overview/index.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/www/src/app/examples/scoped-theme/page.tsx
1 participant
Summary
Screen.Recording.2026-04-30.at.4.15.38.PM.mov
Check usage in
/examples/theme-scopedAny element with
data-theme(or wrapped in<ThemeScope>) creates an isolated theme region. Descendants resolve all--rs-color-*tokens,color-scheme, and the smooth theme-switch transition from the nearest scoped ancestor.The CSS alias chain (
--rs-color-*→--rs-neutral-*→--rs-gray-*) now redeclares each layer on[data-theme="light"]/[data-theme="dark"]so scopes re-anchor cleanly. Adds matchingcolor-schemerules so native UI (form controls, scrollbars) follows the scope.Why a separate
ThemeScope(not extendingThemeProvider)?ThemeProviderbundles global-only concerns: writes todocument.documentElement, localStorage persistence,prefers-color-schemelistener, inline SSR-flash script, and an explicit "nested providers are no-ops" guard. Reusing it for scopes would mean gating every effect on anisScopeflag, removing the nesting guard, and reworking context — same internal split, just behind one export.ThemeScopeis ~30 lines, no state, no effects, does only what scoping needs.Why no
localStorageonThemeScope?Stateless by design. Scopes share no semantic identity, so a single storage key wouldn't fit, and per-scope persistence would reintroduce SSR flash at the scope level (each scope would need its own inline pre-hydration script). Consumers manage state themselves; we deferred a
useScopedThemehelper hook until there's a real ask.Test plan
pnpm testinpackages/raystack(9 new ThemeScope tests pass)/examples/scoped-theme— global and scoped switchers toggle independently; both panels re-theme correctly<div data-theme="dark">inside a light page — native UI (checkboxes, scrollbars) follows the scope🤖 Generated with Claude Code