Repository navigation
🎯 fix: Center Collapsed Sidebar Icons and Unclip Sidebar Focus Rings - #193
Merged
Merged
Conversation
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.
Contributor
Author
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: ba025ac18d
ℹ️ 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".
Contributor
Author
|
Codex Review: Didn't find any major issues. 🚀 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". |
# Conflicts: # src/components/Sidebar.tsx
danny-avila
added a commit
that referenced
this pull request
Oct 29, 2026
…ides Out of the Visual Baselines and Refresh the Two Stale 390 px Ones (#210) e2e/specs.spec.ts reverted the Base spec list only as the last statement of its two saving journeys. When an assertion after Save failed, "E2E Spec" or "E2E Last" stayed in Base, the next run's new spec was named e2e-spec-2, and the serial describe skipped every journey after it. revertList now runs in a beforeAll, for a list an earlier run left behind, and after every journey, passed or failed. It opens a page in a context of its own, so whatever a journey left on its page (a review dialog, unsaved edits) cannot get in the way, returns early when Base stores no list, and checks that the revert took. e2e/import.spec.ts left the Starting credits it imports into Base behind after every live run. They are now reverted after the journey, passed or failed. Every Base override shows in the configuration visual baselines: its tab's override count, the header's revert action at 1440 px, and its chip and section count on its own tab. Each baseline now fails before its screenshot when Base stores an override, naming its tabs. A check alone only covers the moment the page loads: six spec files save to Base in a parallel live run, and a save between a baseline's check and its screenshot reaches the image once the page fetches Base again, as React Query does on window focus when the data is 30 s old. So the cross-worker mkdir lock behind lockUserProfile now also backs a Base lock. Each baseline holds it from loading its tab until its screenshot is taken. Every describe whose journeys save to Base (import, model specs, features, providers, custom endpoint saves, MCP servers) holds it from before the first save until the save is reverted, and releases it after a failure. The import and spec list reverts run as the lock's cleanup. Model specs takes the Base lock before the USER role profile lock, so every worker takes the two in the same order, and the baselines take both, since the System tab also draws a USER profile's override. A leftover therefore never reaches a comparison or an --update-snapshots run. The 390 px system and features baselines failed on main on a clean database. #170 wrote them before its own last changes: at phone width a row's source chip moved under its label, the startup note gained a title and new wording, and "OAuth state TTL (ms)" became "OAuth state TTL". #193 then moved the collapsed rail's icons 1.5 px to its center. Both baselines are regenerated from main on a clean database. No threshold changed. The Base import's cleanup finds its revert by the names revert actions have had since they start with their visible words ("Revert to librechat.yaml (…): Starting credits"); it looked for the old name, found nothing and left the override in Base.
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
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.
Pull Request Template
Summary
This PR supersedes #122 by @dustinhealy and redoes it on current
main. The approach is from #122: collapsed rows withgap-0andjustify-center, zero-width labels, a fixed-width rail row, an unclipped avatar row, and an inset ring on the toggle. #122 doesn't apply as it stands: it conflicts withmaininSidebar.tsxandsrc/test/setup.ts, andmainalready has the click-ui 0.9.1 bump it carried.The sidebar had three problems:
overflow-hidden: 2px on the left and 3.75px at the top and bottom, collapsed and expanded.Changes
gap-0withjustify-center. The labels beside the icons (app title, nav labels, user name and email) becomew-0 flex-none overflow-hidden, so they take no space. The logo, nav icons and avatar center at x=27.5.w-[calc(--spacing(10)-1px)](39px:w-14minus the 1px border andpx-2on each side), from the first frame of the collapse. The icons sit at 27.5 while the panel width animates from 252px to 56px. Withjustify-centeralone, the nav icons start at x=125.5 (the middle of the shrinking panel) and sweep in, and the logo and avatar stay at 26. The expand/collapse animation is unchanged.max-w-full. When the window is too short for the five nav rows, the collapsed nav scrolls and a classic scrollbar takes 6px of its width. The rows then narrow to the space left, so the nav doesn't scroll sideways and the icons stay centered in it.flex items-center gap-2.5 overflow-hidden px-0.5) no longer clips, so the ring stays whole in both states.min-w-0replacesoverflow-hiddenso a long name and email still truncate inside the panel. Without it, a long email runs past the panel's edge.sidebar-toggleclass. A scoped.sidebar-toggle:focus-visible { outline-offset: calc(-1 * var(--focus-ring-width)) !important; }rule, next to the other scoped focus overrides instyles.css, draws the ring inside the button.Not carried over from #122
mainalready has it.renderWithLayoutProviders(src/test/layout-test-utils.tsx), theLayoutRenderOptionstype, and the globalmatchMedia/scrollTo/ResizeObserver/scrollIntoViewstubs insrc/test/setup.ts. The new test renders the sidebar under its own memory router and stubs what it needs, assrc/routes/-tests/app.test.tsxdoes.aria-expandedand the capability-gated nav items, which are outside this fix.-2pxoffset and theborder-radiuson the toggle ring. The offset follows--focus-ring-width, and the toggle stays a square row.w-10(40px) rows. They are 1px wider than the rail's inner width, so the nav icons land at x=28 while the logo and avatar rows shrink back to 39px and land at x=27.5.Change Type
Testing
Unit:
src/components/__tests__/Sidebar.test.tsxrenders the realSidebarunder a memory router with an/_approute:gap-2.5and their labels, and the email is visible.min-w-0).sidebar-toggle, andstyles.cssinsets its focus ring.On
main, the collapsed, both avatar ring and both toggle ring tests fail. The expanded test passes on both.E2E:
e2e/sidebar.spec.tsmeasures the geometry jsdom can't:--hide-scrollbars), and this test fails withoutmax-w-full.On
main, the centering test and all four ring tests fail. The truncation test passes on both and fails ifmin-w-0is dropped (the email then ends at x=381, past the panel's inner edge at x=251). The centering test also fails withjustify-centeralone, without the fixed-width rows.Since #185, the
/_apploader fails closed when the e2e mock backend doesn't serve/api/admin/grants/effective. Against the mock, this spec therefore needs #190. It passes against a live backend withE2E_BASE_URL, with no other changes.Live check: against a LibreChat backend and the production build, in Chromium at 1280×800, light and dark, with keyboard Tab focus. Positions are x offsets from the panel's left edge.
mainThe collapsed rail is the default, so it also appears in the visual baselines in
e2e/a11y/config.spec.ts. The 1440px baselines pass with this PR. On the same stack, the three 390px baselines fail onmaintoo (their page content differs from the stack's config). The 1.5px icon shift adds about 430 differing pixels per image, about 0.1% of the image, well inside the 2% tolerance.Test Configuration:
devbackend, MongoDB 8.2 replica set.bun run build+bun server.ts), Chromium via Playwright.npx playwright test e2e/sidebar.spec.tsagainst the mock backend with 🧪 fix: Serve the Admin's Effective Capabilities from the E2E Mock Backend #190's mock route, and against the live stack withE2E_BASE_URL.npx tsc --noEmit,npx eslint src/ e2e/ --max-warnings 0,npx prettier --checkon the changed files,bun run test:lint-rules(19 pass),npx vitest run(155 files, 3344 tests),bun run build.Checklist