Skip to content

Commit 8a88c1c

Browse files
vivek7405vivek7405
committed
fix(gallery): clear the fixed header when pinning the payload sidebar
Final-review fixes. app/features/layout.ts pins its sidebar at top: 1.5rem, a number its own comment measures against an in-flow navbar. Under this app's fixed header that pins 33px UNDER the bar, so wherever a page is long enough to hold the pin the back link sits behind the blur. The offset is now expressed against --header-h, scoped to the lg breakpoint where the payload shows the aside at all. The sidebar itself is payload and could not be touched. Measured while diagnosing, and worth recording because it looks like this change caused it: the sidebar also rides out of view on a short page, because the payload caps it at calc(100dvh - 7.5rem), leaving it nearly as tall as its own grid row with about 10px of travel. That is identical with the header in flow or out of it, within a pixel, so it is inherent to the payload layout rather than something this branch introduced. Taking the header out of flow was tried and changed nothing, so the header stays fixed. The theme-key fix now has a test. It is a first-paint timing property, which a runner cannot observe reliably (by the time an assertion runs the toggle has upgraded and the two agree), so the invariant is asserted statically on the source, and a second test pins the gallery to the repo-wide key rather than letting it drift back to a private one. Reverting the layout to the old key reds the first test. The @theme versus @theme inline rule reaches the docs site too, which is the surface an app author actually reads. It only landed on the agent skill before, while the docs page still presented input.css as a single plain @theme block and never mentioned that the scaffold emits @theme inline or that the two differ in whether the token reaches :root.
1 parent 2af8351 commit 8a88c1c

3 files changed

Lines changed: 122 additions & 1 deletion

File tree

‎gallery/app/layout.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,25 @@ export default function RootLayout({ children }: { children: unknown }) {
159159
@media (prefers-reduced-motion: reduce) {
160160
*, *::before, *::after { animation-duration: 0.01ms !important; animation-iteration-count: 1 !important; transition-duration: 0.01ms !important; }
161161
}
162+
/* The feature-page sidebar lives in app/features/layout.ts, which is
163+
SCAFFOLD PAYLOAD and cannot be edited from here. It pins with
164+
top: 1.5rem, measured against an in-flow navbar (its own comment says
165+
so), so under this app's fixed header it would pin 33px UNDER the bar
166+
and sit behind the blur wherever the page is long enough to hold the
167+
pin. Re-express the offset against --header-h, which the measure
168+
script keeps current. Scoped to the lg breakpoint because below it the
169+
payload hides the aside and renders its own in-flow back link.
170+
171+
This does NOT make the sidebar hold its pin on a short page. The
172+
payload caps it at calc(100dvh - 7.5rem), so it is nearly as tall as
173+
its own grid row and has about 10px of travel before the row runs out,
174+
measured on /features/components. That is inherent to the payload
175+
layout and is identical with the header in flow or out of it (the back
176+
link lands within 1px either way), so it is not something this change
177+
introduced and not something it can fix from here. */
178+
@media (min-width: 1024px) {
179+
main aside.sticky { top: calc(var(--header-h) + 1.5rem); }
180+
}
162181
</style>
163182
</head>
164183
<body>
@@ -189,7 +208,7 @@ export default function RootLayout({ children }: { children: unknown }) {
189208
<!-- Explicit z-1: a relative element with no z-index does not reliably
190209
sit above the fixed glow layer. -->
191210
<div class="relative z-1">
192-
<main class="min-h-[calc(100dvh-3.5rem)] max-w-5xl mx-auto px-4 sm:px-6 py-8">
211+
<main class="min-h-[calc(100dvh-var(--header-h))] max-w-5xl mx-auto px-4 sm:px-6 py-8">
193212
${children}
194213
</main>
195214
<!-- Written inline rather than extracted to lib/ui/, because

‎test/repo-health/gallery-theme-key.test.mjs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* The gallery's theme bootstrap and its toggle must read and write ONE key.
3+
*
4+
* They disagreed: `gallery/components/theme-toggle.ts` persisted the reader's
5+
* choice under `webjs_theme` while the inline bootstrap in
6+
* `gallery/app/layout.ts` read `theme`. So the bootstrap looked up a key nothing
7+
* ever wrote, found nothing, applied no `data-theme`, and every reader who had
8+
* chosen a theme got a first paint in the OTHER one, then a flash when the
9+
* toggle upgraded and corrected it.
10+
*
11+
* That is a first-paint timing property, which is exactly the shape a test
12+
* runner cannot observe reliably: by the time any assertion runs, the toggle has
13+
* upgraded and the two agree again. So this asserts the invariant STATICALLY,
14+
* on the source, which is both cheap and the only place the defect is visible.
15+
*
16+
* `webjs_theme` is the repo-wide key, not an arbitrary pick. The generated app
17+
* (`packages/cli/lib/create.js`), `examples/blog` and `website/lib/theme.ts`
18+
* (as `THEME_STORAGE_KEY`) all use it, so this also pins the gallery to the
19+
* convention rather than letting it drift back to a private key.
20+
*
21+
* Lives in the REPO suite, not `gallery/test/`, for the reason
22+
* `gallery-favicon.test.mjs` states in its own header: `gallery/test/**` is
23+
* scaffold payload and would be copied into every generated app, where the
24+
* gallery's layout does not exist.
25+
*/
26+
import test from 'node:test';
27+
import assert from 'node:assert/strict';
28+
import { readFileSync } from 'node:fs';
29+
import { resolve, dirname } from 'node:path';
30+
import { fileURLToPath } from 'node:url';
31+
32+
const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..');
33+
const read = (rel) => readFileSync(resolve(REPO_ROOT, rel), 'utf8');
34+
35+
/** Every localStorage key `src` reads or writes, deduped. */
36+
function themeStorageKeys(src) {
37+
const keys = new Set();
38+
for (const m of src.matchAll(/localStorage\.(?:get|set|remove)Item\(\s*'([^']+)'/g)) {
39+
keys.add(m[1]);
40+
}
41+
return [...keys];
42+
}
43+
44+
test('the gallery bootstrap and toggle agree on one theme storage key', () => {
45+
const layoutKeys = themeStorageKeys(read('gallery/app/layout.ts'));
46+
const toggleKeys = themeStorageKeys(read('gallery/components/theme-toggle.ts'));
47+
48+
assert.deepEqual(
49+
layoutKeys,
50+
['webjs_theme'],
51+
"gallery/app/layout.ts's inline bootstrap must read exactly the shared theme key",
52+
);
53+
assert.deepEqual(
54+
toggleKeys,
55+
['webjs_theme'],
56+
'gallery/components/theme-toggle.ts must read and write exactly the shared theme key',
57+
);
58+
// Stated separately from the two above so a future divergence fails on the
59+
// relationship, naming both sides, rather than on whichever file is checked
60+
// first. This is the assertion the flash-of-wrong-theme bug would have tripped.
61+
assert.deepEqual(
62+
layoutKeys,
63+
toggleKeys,
64+
'the bootstrap reads a key the toggle never writes, so a chosen theme paints wrong on first load',
65+
);
66+
});
67+
68+
test('the shared theme key matches every other app in the repo', () => {
69+
// The gallery is the reference a scaffolded app is grown from, so a private
70+
// key here would teach the wrong convention even though nothing would break.
71+
for (const [rel, expected] of [
72+
['packages/cli/lib/create.js', 'webjs_theme'],
73+
['examples/blog/components/theme-toggle.ts', 'webjs_theme'],
74+
]) {
75+
assert.ok(
76+
themeStorageKeys(read(rel)).includes(expected),
77+
`${rel} still uses the shared '${expected}' key`,
78+
);
79+
}
80+
assert.match(
81+
read('website/lib/theme.ts'),
82+
/THEME_STORAGE_KEY\s*=\s*'webjs_theme'/,
83+
"website/lib/theme.ts still exports the shared key as THEME_STORAGE_KEY",
84+
);
85+
});

‎website/app/docs/styling/page.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,23 @@ export default function Styling() {
1212
1313
<p>In dev the scaffold keeps that stylesheet fresh with an on-request recompile (a <code>webjs.dev.regenerate</code> rule), not a background <code>tailwindcss --watch</code>. When a source changes, the dev server recompiles <code>public/tailwind.css</code> before serving it, so a newly added utility class is never rendered unstyled and there is no watch process that can die or lag. Prod builds the same file once before serving, so dev and prod share the identical compile.</p>
1414
15+
<h3><code>@theme</code> and <code>@theme inline</code> are not interchangeable</h3>
16+
<p>Tailwind v4 offers both, the scaffold emits <code>@theme inline</code>, and they differ in one way that is silent when you get it wrong: whether the token reaches <code>:root</code> as a real custom property. Measured on Tailwind 4.3:</p>
17+
18+
<table>
19+
<thead>
20+
<tr><th>Block</th><th>Used only through a utility</th><th>Written as a raw <code>var(--color-x)</code> in scanned source</th><th>Unused</th></tr>
21+
</thead>
22+
<tbody>
23+
<tr><td><code>@theme</code></td><td>emitted</td><td>emitted</td><td>dropped</td></tr>
24+
<tr><td><code>@theme inline</code></td><td><strong>not</strong> emitted, the value is substituted into the utility</td><td>emitted</td><td>dropped</td></tr>
25+
</tbody>
26+
</table>
27+
28+
<p>The cell that bites is <code>inline</code> plus utility-only usage. Nothing on the page can inherit <code>--color-x</code>, so a raw <code>var(--color-x)</code> written somewhere Tailwind never scanned resolves to nothing and the declaration silently falls back to its initial value, which for a border or an outline means <code>currentColor</code>.</p>
29+
30+
<p>Scanned is wider than it looks, and that is the part worth knowing. Tailwind reads source files as raw text, so a <code>var(--color-ring)</code> inside a component's <code>static styles</code> template counts and forces emission, exactly like one written in the stylesheet. So the rule is not that shadow components need a plain <code>@theme</code>. It is: if a token is used only through utilities <em>and</em> something outside the scanned source has to inherit it, map that token with a plain <code>@theme</code>. Anything under a configured <code>@source</code> needs no special handling. Either way a token nothing references is dropped, so an unused mapping is dead configuration rather than a safety net.</p>
31+
1532
<code-block>// public/input.css (compiled to a static public/tailwind.css by css:build,
1633
// which the dev / start tasks run automatically). The @theme maps live here.
1734
@import "tailwindcss";

0 commit comments

Comments
 (0)