Skip to content

Commit f0a1fa1

Browse files
authored
fix(cli): sync scaffold dark mode so ui-* components follow the theme (#101)
* fix(cli): sync scaffold dark mode so ui-* components follow the theme The default scaffold shipped two unsynced theme systems: the editorial chrome tokens (--fg/--bg) toggled via a `data-theme` attribute, while the copied shadcn ui-* components (button, card, alert, etc.) key their dark tokens off a `.dark` class (`@custom-variant dark (&:is(.dark *))`). Nothing ever set `.dark`, so every ui component was frozen in light tokens. On the dark-default chrome this rendered, e.g., the outline "View docs" button as white-on-light (invisible text) and cards as white boxes. Light mode looked fine only because both systems were light then. Fix: the head init script and the theme-toggle now toggle a `.dark` class in sync with the effective theme (dark by default unless the OS prefers light or 'light' is chosen), and the head script re-applies on OS theme changes. The chrome's `data-theme` mechanism is unchanged; the `.dark` class simply drives the shadcn tokens alongside it. Verified in a browser (emulated dark): outline button now renders with the dark input tint + light text, cards render dark, and light mode is unchanged. * docs+test: document and guard the scaffold dark-mode dual-signal Codify why dark mode broke while light worked so it can't recur silently: - agent-docs/styling.md: new "Dark mode" section explaining the two theming signals (data-theme for the editorial chrome, .dark class for the Webjs UI kit), that a theme switch must drive both, and that light mode passing proves nothing about dark mode. - agent-docs/testing.md: a "Verifying UI and theming" note: render in a real browser, test both themes, inspect computed styles, read the screenshot, and guard the wiring in a fast test. - test/scaffolds/scaffold-integration.test.js: assert the generated layout head script AND theme-toggle both toggle the `.dark` class, so the sync mechanism can't regress unnoticed. - templates/CONVENTIONS.md: a scaffolded-app note mirroring the dual-signal rule for agents working inside a user app.
1 parent 52cc3f2 commit f0a1fa1

5 files changed

Lines changed: 95 additions & 5 deletions

File tree

agent-docs/styling.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,34 @@ the definition site and compose naturally with other props (conditional
6363
classes, active states, etc.). They run at SSR time. Output HTML is
6464
identical to inline classes, no client-side runtime.
6565

66+
## Dark mode: two signals, keep them in sync
67+
68+
The default scaffold runs **two** theming systems, and a theme switch must
69+
drive **both** or one half goes stale (this is the single most common
70+
dark-mode bug in a scaffolded app):
71+
72+
1. **Editorial chrome tokens** (`--fg`, `--bg`, `--accent`, ...) declared in
73+
the root layout. They react to a **`data-theme` attribute** on `<html>`
74+
(`data-theme="light"` vs absent) and default to dark.
75+
2. **Webjs UI (shadcn) component tokens** (`--background`, `--foreground`,
76+
`--primary`, ...) used by everything under `components/ui/`. They react
77+
to a **`.dark` class** on an ancestor (`@custom-variant dark (&:is(.dark *))`)
78+
and default to light.
79+
80+
The scaffold's head init script and `theme-toggle` set **both** signals on
81+
`<html>`: they write `data-theme` AND `classList.toggle('dark', isDark)`. If
82+
you wire your own theme switch or replace the toggle, you MUST set both.
83+
Setting only `data-theme` leaves the ui-* components rendering light tokens
84+
on a dark page (white buttons, white cards, invisible text) while the chrome
85+
looks correct.
86+
87+
**Verify dark mode in a real browser, not just light.** Light mode passing
88+
proves nothing about dark mode: with neither signal set, both systems sit at
89+
a coincidentally matching default, so the divergence only appears once
90+
`.dark` / `data-theme` are applied. Emulate dark (`colorScheme: 'dark'` or
91+
flip the toggle) and check a shadcn component's computed `background-color`,
92+
not just the page chrome.
93+
6694
## Vanilla CSS for the whole app (opt-out of Tailwind)
6795

6896
Tailwind isn't required. To hand-write CSS everywhere, you need a

agent-docs/testing.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,26 @@ a feature, the test is probably testing too many things at once.
184184
- **Don't add `.unit` / `.integration` filename suffixes.** The
185185
folder tells you the kind; the filename should match what it
186186
tests.
187+
188+
---
189+
190+
## Verifying UI and theming changes
191+
192+
For anything visual (layout, components, themes), a passing unit test or a
193+
clean-looking code diff is not enough. Render it in a real browser and look.
194+
195+
- **Test both light AND dark mode.** Light mode passing proves nothing about
196+
dark mode. The scaffold drives dark mode through two signals (a `data-theme`
197+
attribute for the editorial chrome and a `.dark` class for the ui-* kit, see
198+
`agent-docs/styling.md`), and when neither is set both default to a
199+
coincidentally matching light, so a desync only shows once dark is active.
200+
Emulate dark (Playwright `newContext({ colorScheme: 'dark' })` or flip the
201+
theme toggle) and inspect a component's **computed** `background-color` /
202+
`color`, not just the page chrome.
203+
- **Read the screenshot.** Capture `page.screenshot({ fullPage: true })` and
204+
open the PNG; white-on-white or a stray light box is obvious visually and
205+
invisible in the markup.
206+
- **Guard the wiring in a fast test where you can.** A runtime cascade bug
207+
needs a browser, but the mechanism that triggers it (e.g. the theme toggle
208+
setting `.dark`) can be asserted cheaply. See the dark-mode assertions in
209+
`test/scaffolds/scaffold-integration.test.js`.

packages/cli/lib/create.js

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -642,10 +642,21 @@ export default function RootLayout({ children }: { children: unknown }) {
642642
<script>
643643
(function(){
644644
try {
645-
var t = localStorage.getItem('webjs_theme');
646-
if (t === 'light' || t === 'dark') {
647-
document.documentElement.dataset.theme = t;
645+
var mq = window.matchMedia('(prefers-color-scheme: light)');
646+
function apply(){
647+
var t = null;
648+
try { t = localStorage.getItem('webjs_theme'); } catch (_) {}
649+
var el = document.documentElement;
650+
if (t === 'light' || t === 'dark') el.dataset.theme = t;
651+
else delete el.dataset.theme;
652+
// Keep shadcn's .dark class in sync with the effective theme so the
653+
// copied ui-* components (button, card, etc.) follow light/dark too.
654+
// Dark is the default unless the OS prefers light or 'light' is set.
655+
var dark = t === 'dark' || (t !== 'light' && !mq.matches);
656+
el.classList.toggle('dark', dark);
648657
}
658+
apply();
659+
mq.addEventListener('change', apply);
649660
} catch (_) {}
650661
})();
651662
</script>
@@ -874,8 +885,13 @@ export class ThemeToggle extends WebComponent {
874885
if (next === 'system') localStorage.removeItem('webjs_theme');
875886
else localStorage.setItem('webjs_theme', next);
876887
} catch {}
877-
if (next === 'system') delete document.documentElement.dataset.theme;
878-
else document.documentElement.dataset.theme = next;
888+
const el = document.documentElement;
889+
if (next === 'system') delete el.dataset.theme;
890+
else el.dataset.theme = next;
891+
// Keep shadcn's .dark class in sync so the ui-* components follow the theme.
892+
const dark = next === 'dark'
893+
|| (next === 'system' && !window.matchMedia('(prefers-color-scheme: light)').matches);
894+
el.classList.toggle('dark', dark);
879895
}
880896
881897
render() {

packages/cli/templates/CONVENTIONS.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,17 @@ or a build-step pipeline. The framework has no hard dependency on Tailwind.
568568
If you mix custom CSS into a light-DOM component, apply the class-prefix
569569
rule (see Components section above).
570570

571+
**Dark mode uses two signals; a theme switch must set both.** The editorial
572+
chrome tokens (`--fg`, `--bg`, `--accent`) follow a `data-theme` attribute on
573+
`<html>`; the Webjs UI kit under `components/ui/` follows a `.dark` class
574+
(`@custom-variant dark (&:is(.dark *))`). The scaffold's head init script and
575+
`theme-toggle` set **both** (`data-theme` AND
576+
`classList.toggle('dark', isDark)`). If you replace the toggle or build your
577+
own theme switch, set both, or the `components/ui/*` render light tokens on a
578+
dark page (white buttons and cards, invisible text) while the chrome looks
579+
correct. Light mode hides this (both systems default to light), so **verify
580+
dark mode in a browser, not just light.**
581+
571582
---
572583

573584
## Styling alternative: vanilla CSS end-to-end

test/scaffolds/scaffold-integration.test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ test('scaffoldApp full-stack: writes the canonical full-stack app layout', async
6363
assert.ok(existsSync(join(appDir, 'app', 'page.ts')), 'page.ts written');
6464
assert.ok(existsSync(join(appDir, 'components', 'theme-toggle.ts')), 'theme-toggle written');
6565

66+
// Dark-mode wiring: the head init script (in layout) AND the theme-toggle
67+
// must toggle the shadcn `.dark` class, not only the `data-theme`
68+
// attribute. Without `.dark`, the copied components/ui/* render light
69+
// tokens on the dark chrome (white buttons/cards, invisible text). Light
70+
// mode hides this, so guard it here. See agent-docs/styling.md "Dark mode".
71+
const layoutSrc = readFileSync(join(appDir, 'app', 'layout.ts'), 'utf8');
72+
const toggleSrc = readFileSync(join(appDir, 'components', 'theme-toggle.ts'), 'utf8');
73+
assert.match(layoutSrc, /classList\.toggle\(['"]dark['"]/,
74+
'layout head script must toggle the .dark class (shadcn dark-mode signal)');
75+
assert.match(toggleSrc, /classList\.toggle\(['"]dark['"]/,
76+
'theme-toggle must toggle the .dark class (shadcn dark-mode signal)');
77+
6678
// Prisma + lib singleton wired up
6779
assert.ok(existsSync(join(appDir, 'prisma', 'schema.prisma')), 'prisma schema written');
6880
assert.ok(existsSync(join(appDir, 'lib', 'prisma.server.ts')), 'lib/prisma.server.ts written');

0 commit comments

Comments
 (0)