Repository navigation
🎯 fix: Center Collapsed Sidebar Icons and Unclip Sidebar Focus Rings - #122
dustinhealy wants to merge 4 commits into
Conversation
Inline @clickhouse/click-ui in vitest server deps and exclude .claude/** from test discovery. click-ui 0.9.1 ships ESM that imports .css files, which Node's loader cannot parse, so Vitest must process the package through Vite.
The collapsed sidebar kept its labels mounted with a gap-2.5 that never collapsed, so the 10px gap next to the zero-width truncated labels pushed the logo, nav icons, and avatar left of center and justify-* had no effect. Rows now drop to gap-0 with justify-center when collapsed, and the user row's flex-1 label container becomes w-0 flex-none so it cannot grow into the free space and offset the avatar. The user row wrapper's overflow-hidden clipped the avatar button's focus ring, which the global convention draws at a 2px offset outside the box. The wrapper no longer clips; the name and email already truncate inside their min-w-0 container and the aside still clips during the width transition. The collapse toggle is w-full and flush with the panel edges and the viewport bottom, so ancestor clipping leaves only the top segment of an outside ring visible. A scoped sidebar-toggle override draws the focus outline inset instead.
dustinhealy
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ba92ba09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e real sidebar The collapsed centering switched to w-full justify-center while the aside width was still animating between w-63 and w-14, so the logo, nav icons, and avatar jumped toward the middle of the still-wide sidebar and swept back as it narrowed. A w-10 rail matches the collapsed content width exactly (w-14 minus the px-2 container padding), so icons land in their final centered position immediately and never reflow during the width transition. The sidebar tests now live under src/components/__tests__ per the repo convention and render the real component through a shared layout-test-utils helper with the real router, click-ui provider, theme provider, query client, and i18n, mocking only the server function boundary. This exercises the real dropdown trigger wrapper structure and capability gating instead of asserting classes on mocked stand-ins.
|
@codex re-review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daf6611adf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Locally defined interfaces live in src/types and are consumed through the types namespace import per the repo convention, so the test helper's options type moves next to SidebarProps in layout.ts.
|
@codex re-review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…193) * 🎯 fix: Center Collapsed Sidebar Icons and Unclip Sidebar Focus Rings The collapsed rail drew the logo, nav icons and avatar at x=26, 1.5px left of the rail's inner center (27.5): the rows lay out from the left in both states, so the icons stayed where the expanded panel puts them. Collapsed rows now use gap-0 with justify-center, and the labels beside the icons (app title, nav labels, user name and email) become w-0 flex-none overflow-hidden so they take no space. The rows are also the rail's inner width (39px) from the first frame of the collapse, so the icons sit at 27.5 while the panel width animates instead of sweeping in from the middle of the shrinking panel. The avatar's focus ring (2px at a 2px offset) was cut by its row wrapper's overflow-hidden. The wrapper no longer clips; min-w-0 keeps a long name and email truncating inside the panel. The collapse toggle is flush with the panel's clipped edges and the bottom of the viewport, so only the top of its ring showed. A scoped .sidebar-toggle:focus-visible rule draws the ring inset. Supersedes #122 by @dustinhealy, whose approach this follows. * 📏 fix: Let Collapsed Rail Rows Narrow Beside a Scrolling Nav's Scrollbar
|
Thanks @dustinhealy for this. #193 is now merged. It redoes this PR on current This PR no longer applied as it stood: it conflicted with Closing this in favor of #193. If you spot a sidebar layout it misses, please comment or open an issue. |
Summary
Three sidebar polish issues, all pre-existing (introduced by the sidebar collapse refactor in #6, not by the click-ui 0.9.1 bump):
gap-2.5between icon and label never collapsed and the truncated label spans kept a shrink-to-fit sliver of width, sojustify-centerhad nothing to center. Collapsed rows now usegap-0withjustify-center, and every label span (logo title, nav labels, user-row name/email container) becomesw-0 flex-none overflow-hiddenwhile collapsed so no residual width orflex-1growth can absorb the free space. Measured result: icon centers moved from x=26.0 (logo x=20.0) to exactly x=27.5, the true center of the 55px inner rail.outline-offset: 2pxoutside the box, and the row wrapper'soverflow-hiddencut it on all sides. The wrapper clip is removed; the labels already truncate and the aside still clips during the width transition..sidebar-toggle:focus-visiblerule draws the ring inset (outline-offset: -2px), following the existing scoped-override convention instyles.css.Change Type
Testing
New
Sidebar.test.tsx(3 cases) asserts the collapsed-state classes (justify-center,gap-0, nooverflow-hiddenon the user row,sidebar-toggleon the toggle) and that the expanded state restoresgap-2.5. Geometry itself is not assertable in jsdom; visual verification below.Manually verified in the app: collapsed icons center in the rail, the avatar focus ring is a full circle, and tabbing to the collapse toggle shows a complete ring.
Before/after proof:
Test Configuration:
Checklist