Skip to content

fix(sidebar): SW-2097 keep collapsed SidebarSeparator within the rail #154

Open
boramyi-ts wants to merge 1 commit into
mainfrom
SW-2097-sidebar-primitive-separator
Open

fix(sidebar): SW-2097 keep collapsed SidebarSeparator within the rail #154
boramyi-ts wants to merge 1 commit into
mainfrom
SW-2097-sidebar-primitive-separator

Conversation

@boramyi-ts

Copy link
Copy Markdown
Contributor

Sidebar primitive — collapsed separator overflows the rail

In the ui/sidebar.tsx shadcn primitive, SidebarSeparator overrode the base Separator's data-horizontal:w-full with a plain w-auto. tailwind-merge keys those under different variants, so both classes survived and data-horizontal:w-full won — the divider rendered at full rail width plus its mx-2 margins, spilling ~8px past the right edge. In the collapsed icon rail (Components/Sidebar → CollapsedIcon) the line visibly crossed the rail border into the content area and no longer aligned under the icon column.

Measured (collapsed, 47px rail): separator was x: 8 → 55 (width 47, +8px past the rail's right edge of 47).

Fix

Override on the matching variant so tailwind-merge dedupes it:

- className={cn("mx-2 w-auto bg-sidebar-border", className)}
+ className={cn("mx-2 data-horizontal:w-auto bg-sidebar-border", className)}

width: auto (unlike w-full) subtracts the mx-2 margins, so the line sits inset and centered. After: separator is x: 8 → 39 (width 31), centered in the rail under the icon column. This also corrects the (less obvious) slight overflow in the expanded sidebar.

Before / After (collapsed rail, zoomed)

Before After
before after

In before the line crosses the rail's right border into the content area; in after it's inset within the rail, aligned under the icons.

Verification

  • yarn lint — clean (zero warnings)
  • yarn typecheck — clean
  • yarn test:storybook sidebar — 15/15 pass

Note

Sibling to #153 (SW-2097), which fixes the analogous full-width-divider issue in the DataAppShell composed component. This PR targets the shadcn Sidebar primitive specifically.

🤖 Generated with Claude Code

`SidebarSeparator` tried to override the base `Separator`'s
`data-horizontal:w-full` with a plain `w-auto`. tailwind-merge keys those
under different variants, so both survived and `w-full` won — the divider
rendered at full rail width *plus* its `mx-2` margins, spilling ~8px past
the right edge. Most visible in the collapsed icon rail, where the line
crossed the rail border into the content area.

Override on the matching variant (`data-horizontal:w-auto`) so it dedupes
against `data-horizontal:w-full`. `width: auto` then subtracts the `mx-2`
margins, leaving the separator inset and centered under the icon column.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boramyi-ts boramyi-ts requested review from a team as code owners June 23, 2026 23:09
@boramyi-ts boramyi-ts temporarily deployed to artifactory-prod June 23, 2026 23:09 — with GitHub Actions Inactive
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ts-lib-ui-kit-storybook Ignored Ignored Jun 23, 2026 11:09pm

Request Review

@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 94.74% (🎯 83%)
🟰 ±0%
20627 / 21770
🟢 Statements 94.74% (🎯 83%)
🟰 ±0%
20627 / 21770
🟢 Functions 93.58% (🎯 74%)
🟰 ±0%
919 / 982
🟢 Branches 88.74% (🎯 81%)
🟰 ±0%
3770 / 4248
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/components/ui/sidebar.tsx 94.28%
🟰 ±0%
77.27%
🟰 ±0%
96.15%
🟰 ±0%
94.28%
🟰 ±0%
50-51, 80, 99-106, 183-206, 563
Generated in workflow #832 for commit 0c7920d by the Vitest Coverage Report Action

@boramyi-ts boramyi-ts changed the title fix(sidebar): keep collapsed SidebarSeparator within the rail [SW-2097] fix(sidebar): SW-2097 keep collapsed SidebarSeparator within the rail Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants