Skip to content

Commit c65198d

Browse files
Applied CodeRabbit suggestions
1 parent fb85732 commit c65198d

8 files changed

Lines changed: 147 additions & 64 deletions

File tree

‎AGENTS.md‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,20 @@ in `out/` is served by any static host (GitHub Pages, GitLab Pages, Bitbucket).
1111
**Key Features:**
1212
- Search over curated repository listing (`repositories.txt`)
1313
- Dark mode, 100% CSS switching (`components/theme-toggle.jsx` +
14-
`plans/css-only-dark-mode.md`): three radios (light/dark/system) on a
15-
right-aligned `.theme-bar` row below the header, rendered by
16-
`app/layout.jsx` between `<SiteHeader>` and the persistence script (the
17-
right-edge mirror of the breadcrumbs' left-edge alignment), palettes
18-
selected in `app/globals.css` via `:root:has(#rf-theme-…:checked)` +
14+
`plans/css-only-dark-mode.md`): three radios (light/dark/system) on the
15+
right side of the `.theme-bar` row below the header, rendered by
16+
`app/layout.jsx` — the breadcrumb trail shares the row on report routes
17+
(crumbs left, toggle right, vertically centered; the right-edge mirror
18+
of the breadcrumbs' left-edge alignment) — palettes selected in
19+
`app/globals.css` via `:root:has(#rf-theme-…:checked)` +
1920
`prefers-color-scheme`; an inline pre-paint script in `app/layout.jsx`
2021
only restores/persists the choice (CSP-hashed by the build). Guards:
2122
`tests/unit/theme-css.test.js` (the two dark blocks must stay
2223
byte-identical; all hex colors live in the palette blocks; dark palette
2324
passes the same WCAG AA math as `tests/unit/css-a11y.test.js`)
24-
- Breadcrumb trail under the menu (`components/breadcrumbs.jsx` +
25-
`lib/breadcrumbs.js`): mirrors `/user/repo/branch` with linked ancestors;
25+
- Breadcrumb trail on the theme bar row under the menu
26+
(`components/breadcrumbs.jsx` + `lib/breadcrumbs.js`): mirrors
27+
`/user/repo/branch` with linked ancestors;
2628
the repo crumb never self-links, and ReportView announces the resolved
2729
branch (`rf:branch-resolved` event) so the label matches the loaded report
2830
(`main` 404 → `master` fallback)

‎app/globals.css‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -352,20 +352,20 @@ pre {
352352

353353
/* ---- Breadcrumbs (user/report trail under the top menu) ---- */
354354
.breadcrumbs {
355-
/* Same column as the top menu: #top-menu is capped by mvp.css at
356-
--width-content, so reuse the variable — the two can never drift apart.
357-
border-box (not mvp's default content-box) so the horizontal padding
358-
insets crumbs exactly like .menu-bar's padding insets the brand; with
359-
content-box, symmetric padding on a centered block never moves the
360-
content edge. */
361-
max-width: var(--width-content);
362-
box-sizing: border-box;
355+
/* Lives on the .theme-bar row (rendered by app/layout.jsx), sharing the
356+
line with the color theme toggle: the row owns the menu-column
357+
alignment formula (max-width: var(--width-content) + 0.8rem horizontal
358+
padding, see .theme-bar above), so the trail must add no box-model
359+
offsets of its own — flex-grow fills the row's left side, keeping the
360+
first crumb's glyph at the menu bar's left content edge while the
361+
toggle pins to the right edge, both vertically centered. */
362+
flex: 1 1 auto;
363+
min-width: 0;
363364
/* mvp.css gives bare nav a 7rem bottom margin, bold text and a spaced flex
364-
row — reset all three so the trail hugs the menu like a sub-bar. */
365-
margin: 0 auto;
365+
row — reset all three so the trail sits inline like a sub-bar. */
366+
margin: 0;
366367
font-weight: normal;
367368
display: block;
368-
padding: 0 0.8rem;
369369
font-size: 0.85rem;
370370
}
371371

@@ -685,14 +685,19 @@ pre {
685685
}
686686

687687
/* ---- Color theme toggle (three radio modes, pure-CSS switching) ---- */
688-
/* Row below the site header (rendered by app/layout.jsx): the same alignment
689-
formula .breadcrumbs use (--width-content column, 0.8rem horizontal
690-
padding, border-box) keeps the toggle's right edge flush with the menu
691-
bar's right content edge at every viewport — the right-edge mirror of the
692-
breadcrumb trail's left-edge alignment. #top-menu is capped by mvp.css at
693-
--width-content, so the row and the menu bar can never drift apart. */
688+
/* Row below the site header (rendered by app/layout.jsx): the breadcrumb
689+
trail (when the route has one) on the left and the toggle on the right,
690+
vertically centered on one line. The alignment formula .breadcrumbs used
691+
to carry lives here now: --width-content column, 0.8rem horizontal
692+
padding, border-box — keeping the first crumb's glyph at the menu bar's
693+
left content edge and the toggle at its right content edge at every
694+
viewport, the two edges mirroring each other. #top-menu is capped by
695+
mvp.css at --width-content, so the row and the menu bar can never drift
696+
apart. Crumb routes grow the trail (flex: 1 1 auto); crumb-less routes
697+
leave only the toggle, pinned right by flex-end. */
694698
.theme-bar {
695699
display: flex;
700+
align-items: center;
696701
justify-content: flex-end;
697702
max-width: var(--width-content);
698703
box-sizing: border-box;

‎app/layout.jsx‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,20 @@ export default function RootLayout({ children }) {
4949
</head>
5050
<body>
5151
<SiteHeader repositories={repositories} />
52-
{/* Color theme toggle on a row below the header: .theme-bar reuses
53-
the breadcrumbs' alignment formula (--width-content column,
54-
0.8rem horizontal padding, border-box) so the toggle's right
55-
edge sits flush with the menu bar's right content edge,
56-
mirroring how the breadcrumb trail aligns with the bar's left
57-
edge (plans/css-only-dark-mode.md). */}
52+
{/* Theme bar row below the header: the breadcrumb trail (report/user
53+
routes only) on the left and the color theme toggle on the right,
54+
vertically centered on one line. The row reuses the breadcrumbs'
55+
alignment formula (--width-content column, 0.8rem horizontal
56+
padding, border-box), so the first crumb's glyph sits at the menu
57+
bar's left content edge and the toggle at its right content edge
58+
— the two edges mirror each other (plans/css-only-dark-mode.md).
59+
Suspense is required for useSearchParams (static export; the
60+
fallback keeps prerendered HTML crumb-free and the toggle still
61+
renders — an empty flex slot). */}
5862
<div className="theme-bar">
63+
<Suspense fallback={null}>
64+
<Breadcrumbs />
65+
</Suspense>
5966
<ThemeToggle />
6067
</div>
6168
{/* Theme persistence shim (plans/css-only-dark-mode.md): switching
@@ -74,12 +81,6 @@ export default function RootLayout({ children }) {
7481
"(function(){var r;try{r=localStorage.getItem('rf-theme')}catch(e){}if(r==='light'||r==='dark'){var i=document.getElementById('rf-theme-'+r);if(i)i.checked=true}document.addEventListener('change',function(e){var t=e.target;if(t&&t.name==='rf-theme'){try{localStorage.setItem('rf-theme',t.value)}catch(_){}}})})();"
7582
}}
7683
/>
77-
{/* Breadcrumb trail under the menu (report/user routes only). Suspense
78-
is required because useSearchParams deopts otherwise (static
79-
export; the fallback keeps prerendered HTML crumb-free). */}
80-
<Suspense fallback={null}>
81-
<Breadcrumbs />
82-
</Suspense>
8384
<main id="app" tabIndex={-1}>{children}</main>
8485
<SiteFooter />
8586
<SentryProvider />

‎plans/css-only-dark-mode.md‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ renders three radios named `rf-theme` (`rf-theme-light`, `rf-theme-dark`,
2121
icon pill (inline SVG: sun / moon / monitor). The toggle sits on the
2222
**`.theme-bar` row below `#top-menu`**, rendered by `app/layout.jsx` right
2323
after `<SiteHeader />` (and before the persistence shim, which needs the
24-
radios parsed), right-aligned with the menu bar's content edge.
24+
radios parsed); on report routes the breadcrumb trail shares the row, to the
25+
toggle's left, both vertically centered, the toggle pinned to the menu bar's
26+
right content edge.
2527

2628
Why radios: "one of three modes" is exactly a radio group, the checked state
2729
is queryable from CSS (`:has(#id:checked)`), and switching requires zero JS.
@@ -124,15 +126,16 @@ practices" and "only CSS" conflict here, so the plan resolves it as:
124126
### Alignment with the menu
125127

126128
The toggle lives on a `.theme-bar` row rendered by `app/layout.jsx` right
127-
after `<SiteHeader />`, below the header block, pinned right via
128-
`justify-content: flex-end`. The row reuses the breadcrumbs' exact alignment
129-
formula — `max-width: var(--width-content)`, `box-sizing: border-box`,
130-
`margin: 0 auto`, `padding: 0.4rem 0.8rem 0.45rem` (see
131-
`tests/unit/layout-styles.test.js` for the breadcrumb invariant this mirrors)
132-
— so its right content edge is the menu bar's right content edge at every
133-
viewport, with no positioning tricks or magic numbers; the formula works
134-
identically at mobile widths, where the row simply follows the wrapped menu
135-
bar.
129+
after `<SiteHeader />`, below the header block. The row owns the alignment
130+
formula the standalone breadcrumb bar used to carry — `max-width:
131+
var(--width-content)`, `box-sizing: border-box`, `margin: 0 auto`,
132+
`padding: 0.4rem 0.8rem 0.45rem` — with `align-items: center` so the
133+
breadcrumb trail (report/user routes; `flex: 1 1 auto`, no box-model offsets
134+
of its own) sits on the line's left at the menu bar's left content edge,
135+
vertically centered against the toggle pinned to the right content edge
136+
(`justify-content: flex-end` keeps the toggle right on crumb-less pages).
137+
No positioning tricks or magic numbers; the formula works identically at
138+
mobile widths, where the row simply follows the wrapped menu bar.
136139

137140
### Toggle UI / a11y
138141

‎tests/e2e/dark-mode.spec.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@
33
// (radio :has() + prefers-color-scheme), and the inline bootstrap script
44
// persists the choice across hard loads.
55
import { test, expect } from '@playwright/test';
6+
import { readFileSync } from 'node:fs';
7+
import { fileURLToPath } from 'node:url';
8+
import { dirname, join } from 'node:path';
9+
10+
const SAMPLE_REPORT = readFileSync(
11+
join(dirname(fileURLToPath(import.meta.url)), '../fixtures/junit4-report.json'),
12+
'utf8'
13+
);
614

715
const LIGHT_BG = 'rgb(255, 255, 255)';
816
const DARK_BG = 'rgb(16, 22, 29)';
@@ -119,3 +127,38 @@ test('dark mode keeps rendering dark on report pages after navigation', async ({
119127
expect(await bodyBackground(page)).toBe(DARK_BG);
120128
expect(await page.evaluate(() => localStorage.getItem('rf-theme'))).toBe('dark');
121129
});
130+
131+
test('breadcrumbs and the toggle share one vertically centered row on report pages', async ({ page }) => {
132+
test.skip(test.info().project.name !== 'chromium', 'geometry check runs once');
133+
// Mock GitHub raw content so the report page never hits the network.
134+
await page.route('**/raw.githubusercontent.com/**', route => {
135+
if (route.request().url().endsWith('refactor-first.json')) {
136+
route.fulfill({ status: 200, contentType: 'application/json', body: SAMPLE_REPORT });
137+
} else {
138+
route.continue();
139+
}
140+
});
141+
await page.goto('/refactorfirst/refactorfirst/master/');
142+
await page.waitForLoadState('networkidle');
143+
const edges = await page.evaluate(() => {
144+
const crumb = document.querySelector('nav[aria-label="Breadcrumb"] ol li a');
145+
const crumbBox = crumb.getBoundingClientRect();
146+
const toggle = document.querySelector('.theme-toggle').getBoundingClientRect();
147+
const bar = document.querySelector('.menu-bar').getBoundingClientRect();
148+
return {
149+
crumbGlyphLeft: crumbBox.left + parseFloat(getComputedStyle(crumb).paddingLeft),
150+
brandLeft: document.querySelector('.brand').getBoundingClientRect().left,
151+
sameRow: crumbBox.top < toggle.bottom && toggle.top < crumbBox.bottom,
152+
centerDelta: Math.abs((crumbBox.top + crumbBox.bottom) / 2 - (toggle.top + toggle.bottom) / 2),
153+
toggleRight: toggle.right,
154+
barContentRight: bar.right - parseFloat(getComputedStyle(document.querySelector('.menu-bar')).paddingRight)
155+
};
156+
});
157+
// The first crumb's glyph keeps the menu column's left content edge…
158+
expect(Math.abs(edges.crumbGlyphLeft - edges.brandLeft)).toBeLessThanOrEqual(1);
159+
// …the toggle keeps the right content edge…
160+
expect(Math.abs(edges.toggleRight - edges.barContentRight)).toBeLessThanOrEqual(1);
161+
// …and the two share one vertically centered line.
162+
expect(edges.sameRow).toBe(true);
163+
expect(edges.centerDelta).toBeLessThanOrEqual(1);
164+
});

‎tests/integration/theme-toggle.test.jsx‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
// Dark mode toggle below the site header: three radio modes (light / dark /
22
// system) rendered as icon pills, with the system preference preselected and
3-
// the whole control on a right-aligned .theme-bar row under the header whose
4-
// right edge aligns with the menu bar's content edge
3+
// the whole control on a right-aligned .theme-bar row under the header —
4+
// the row the breadcrumb trail shares on report routes (crumbs left, toggle
5+
// right), aligned with the menu bar's content edges
56
// (plans/css-only-dark-mode.md).
67
import { describe, test, expect } from 'bun:test';
78
import { mock } from 'bun:test';
89

910
import { sharedNextNavigationMock } from './next-navigation-stub';
1011

12+
let currentPath = '/';
13+
let currentSearch = '';
1114
mock.module('next/navigation', () => sharedNextNavigationMock({
12-
usePathname: () => '/',
13-
useSearchParams: () => new URLSearchParams()
15+
usePathname: () => currentPath,
16+
useSearchParams: () => new URLSearchParams(currentSearch)
1417
}));
1518

1619
import { render, installRtlDom, within } from './rtl';
@@ -82,4 +85,27 @@ describe('Root layout placement', () => {
8285
const shim = html.indexOf("localStorage.getItem('rf-theme')");
8386
expect(shim).toBeGreaterThan(themeBar);
8487
});
88+
89+
test('on report routes the breadcrumb trail shares the toggle row, to its left', () => {
90+
currentPath = '/alice/some-repo/master';
91+
currentSearch = '';
92+
try {
93+
const html = renderToStaticMarkup(_jsx(RootLayout, { children: null }));
94+
const themeBar = html.indexOf('class="theme-bar"');
95+
const main = html.indexOf('<main');
96+
const rowHtml = html.slice(themeBar, main);
97+
const crumbs = rowHtml.indexOf('aria-label="Breadcrumb"');
98+
const toggle = rowHtml.indexOf('rf-theme-light');
99+
expect(crumbs, 'breadcrumb nav missing from the theme bar').toBeGreaterThan(-1);
100+
expect(toggle).toBeGreaterThan(-1);
101+
// DOM order mirrors the flex row: crumbs first (left), toggle last
102+
// (right); the row's flex-grow keeps them on one line.
103+
expect(crumbs).toBeLessThan(toggle);
104+
// The trail no longer renders outside the row.
105+
expect(html.slice(0, themeBar)).not.toContain('aria-label="Breadcrumb"');
106+
expect(html.slice(main)).not.toContain('aria-label="Breadcrumb"');
107+
} finally {
108+
currentPath = '/';
109+
}
110+
});
85111
});

‎tests/unit/layout-styles.test.js‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,18 @@ describe('layout styles (mvp.css section-flex reset)', () => {
5656
expect(body).toMatch(/margin:\s*0\s+auto\s*;/);
5757
});
5858

59-
test('the breadcrumb bar shares the top menu column (#top-menu caps)', () => {
60-
// #top-menu is capped by mvp.css at --width-content; reusing the variable
61-
// keeps the breadcrumb column and the menu column aligned forever. The
62-
// bar must be border-box so its horizontal padding insets crumbs exactly
63-
// like .menu-bar's padding insets the brand: with content-box, symmetric
64-
// padding on a centered block never moves the content edge.
59+
test('the breadcrumb trail stretches inside the theme bar row (menu column)', () => {
60+
// The trail now shares the .theme-bar row with the color theme toggle:
61+
// the row carries the alignment formula (max-width: var(--width-content)
62+
// + 0.8rem horizontal padding, guarded in tests/unit/theme-css.test.js),
63+
// so .breadcrumbs must not add its own box-model offsets — it grows to
64+
// fill the row's left side, keeping the first crumb's glyph at the menu
65+
// column's left content edge while the toggle pins to the right edge.
6566
const body = rules.get('.breadcrumbs');
6667
expect(body).toBeTruthy();
67-
expect(body).toMatch(/max-width:\s*var\(--width-content\)/);
68-
expect(body).toMatch(/box-sizing:\s*border-box\s*;/);
69-
expect(body).toMatch(/padding:\s*0\s+0\.8rem\s*;/); // matches .menu-bar
68+
expect(body).toMatch(/flex:\s*1\s+1\s+auto/);
69+
expect(body).not.toMatch(/max-width/);
70+
expect(body).not.toMatch(/padding:\s*0\s+0\.8rem/);
7071
});
7172

7273
test('crumb links carry no left padding (glyph edge aligns with the brand)', () => {

‎tests/unit/theme-css.test.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,15 +224,17 @@ describe('toggle styles', () => {
224224
expect(height).toBeGreaterThanOrEqual(1.5);
225225
});
226226

227-
it('the sub-row under the menu bar mirrors the breadcrumb alignment formula', () => {
228-
// Same invariant layout-styles.test.js pins for .breadcrumbs: the row
229-
// shares #top-menu's --width-content column and the 0.8rem horizontal
230-
// padding insets the toggle to the menu bar's right *content* edge at
231-
// every viewport. Flex-end pins the control to that edge.
227+
it('the row below the header mirrors the breadcrumb alignment formula', () => {
228+
// Same invariant layout-styles.test.js pinned for the standalone
229+
// .breadcrumbs bar: the row shares #top-menu's --width-content column
230+
// and the 0.8rem horizontal padding insets its children to the menu
231+
// bar's content edges at every viewport — the toggle on the right, the
232+
// breadcrumb trail on the left (flex-grow), vertically centered.
232233
const row = rules.get('.theme-bar');
233234
expect(row).toBeTruthy();
234235
expect(row).toMatch(/display:\s*flex/);
235236
expect(row).toMatch(/justify-content:\s*flex-end/);
237+
expect(row).toMatch(/align-items:\s*center/);
236238
expect(row).toMatch(/max-width:\s*var\(--width-content\)/);
237239
expect(row).toMatch(/box-sizing:\s*border-box/);
238240
expect(row).toMatch(/margin:\s*0\s+auto/);

0 commit comments

Comments
 (0)