🎯 fix: Center Collapsed Sidebar Icons and Unclip Sidebar Focus Rings - #122
Open
dustinhealy wants to merge 4 commits into
Open
🎯 fix: Center Collapsed Sidebar Icons and Unclip Sidebar Focus Rings#122dustinhealy wants to merge 4 commits into
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
dustinhealy
marked this pull request as ready for review
August 11, 2026 23:03
Contributor
Author
|
@codex review |
dustinhealy
marked this pull request as draft
August 14, 2026 15:03
![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.
Contributor
Author
|
@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.
Contributor
Author
|
@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". |
dustinhealy
marked this pull request as ready for review
August 14, 2026 16:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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