fix(sidebar): clean up collapsed header panel layout#2878
Conversation
PR #2872 clipped the sidebar body when collapsed but its companion HeaderRow change (shipped "not run locally") left the header's left panel — the strip holding the Bluebird button and sidebar toggle — in a broken state when collapsed: - On macOS the traffic-light clearance was added to the panel's width while the buttons stayed right-aligned, so the wider Bluebird+toggle cluster drifted back under the window controls. - The panel kept its right border when collapsed, leaving a divider dangling in the header with no sidebar body beneath it — reading as a leftover sidebar stub floating over the page content. Fix: extract the panel layout into a pure, testable `getHeaderSidebarPanelLayout` helper. When collapsed it now reserves the macOS traffic-light strip as real left padding and left-aligns the buttons (so a wide label grows rightward into empty space, never leftward under the controls), and drops the divider (which only belongs while the sidebar body is visible below it). Open-state behavior is unchanged. Verified: typecheck, biome check, and 6 new unit tests covering all open/collapsed × macOS/non-macOS states pass. Generated-By: PostHog Code Task-Id: f4b5e35b-f65b-4c17-a78a-f7076a8a271c
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(sidebar): clean up collapsed header ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("never reserves traffic-light padding while the sidebar is open", () => { | ||
| expect( | ||
| getHeaderSidebarPanelLayout({ | ||
| sidebarOpen: true, | ||
| sidebarWidth: 256, | ||
| isMac: true, | ||
| }).paddingLeft, | ||
| ).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("drops the divider when collapsed so it can't dangle over the page", () => { | ||
| for (const isMac of [true, false]) { | ||
| expect( | ||
| getHeaderSidebarPanelLayout({ | ||
| sidebarOpen: false, | ||
| sidebarWidth: 256, | ||
| isMac, | ||
| }).showBorder, | ||
| ).toBe(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Both standalone it() guards test assertions already fully covered by the it.each block above: the "open on macOS" and "open off macOS" cases already assert paddingLeft: undefined, and both collapsed cases already assert showBorder: false. Keeping them adds noise without adding failure coverage (simplicity rule 4: no superfluous parts).
Additionally, the for (const isMac of [true, false]) loop in the second guard is exactly the pattern the team prefers to express as a parameterised .each — though since the cases are already covered above, the cleanest fix is simply to remove both standalone tests.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The two standalone it() guards duplicated assertions already covered by the it.each block (which asserts the full layout object via toEqual for every state), and one used a for-loop where the team prefers it.each. Remove them and fold the regression intent (traffic-light clearance, dropped divider) into the parameterized case names instead. Per Greptile review feedback on #2878. Generated-By: PostHog Code Task-Id: f4b5e35b-f65b-4c17-a78a-f7076a8a271c
2 participants
Problem
When the sidebar is collapsed, the header's left panel — the strip holding the Bluebird button and the sidebar toggle — renders broken. Reported symptoms:
These come from the
HeaderRow.tsxportion of #2872 (which, per its own description, was "not run locally"). The sidebar body is correctly clipped to 0 width there, but the header panel's collapse logic was off:justify="end"). Because the Bluebird + toggle cluster is wider than the base collapsed width, right-aligning let its left edge drift back under the traffic lights.Screen.Recording.2026-06-23.at.17.14.09.mov
Changes
headerSidebarPanel.ts(new): pure, testablegetHeaderSidebarPanelLayout({ sidebarOpen, sidebarWidth, isMac })that derives width / minWidth / paddingLeft / justify / border. When collapsed it reserves the macOS traffic-light strip as real left padding and left-aligns the buttons (a wide "Bluebird" label now grows rightward into empty space, never leftward under the controls), and drops the divider (it only belongs while the sidebar body is visible below it). Open-state behavior is unchanged.HeaderRow.tsx: uses the helper instead of inline style math; the panel constants moved into the helper.headerSidebarPanel.test.ts(new): parameterized tests across all open/collapsed × macOS/non-macOS states, plus regression guards (no traffic-light padding while open; no divider when collapsed).How did you test this?
pnpm --filter @posthog/ui typecheck— cleannpx biome checkon the changed files — cleanvitest— 6/6 passCould not run the Electron GUI in the dev sandbox (no display / nothing on CDP), so the layout logic is covered by unit tests + static analysis. Worth a quick manual check: collapse the sidebar on macOS and confirm Bluebird clears the traffic lights and no divider/stub remains over the content.
🤖 Generated with Claude Code