fix: sidebar spacing and group margin#812
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refactors the sidebar component's spacing and layout system across three coordinated changes. The CSS module updates define a new spacing strategy: adding gap spacing to the root container, simplifying header padding to a two-value format, removing the previous gap from the main container, and introducing margin-top spacing between consecutive navigation groups. The SidebarMain component is updated to include a gap={3} prop on its Flex element to match this new layout structure. Finally, all Sidebar demo code snippets are updated to include explicit inline padding styling within the header Flex elements, showing the updated pattern consistently across documentation examples. Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/www/src/content/docs/components/sidebar/demo.ts (1)
26-26: ⚡ Quick winConsider refactoring to avoid requiring inline styles in demos.
All demo examples now require
style={{padding:"4px"}}on theFlexelement insideSidebar.Header. This pattern has several drawbacks:
- The hardcoded
"4px"doesn't reference design system tokens, creating potential inconsistency- Developers copying demo code must remember to include the inline style or spacing will be incorrect
- Changes to this spacing value would require updating all demos
Consider one of these alternatives:
- Adjust
.headerCSS to handle content padding by default- Create a
Sidebar.HeaderContentwrapper component with appropriate spacing- Make the padding a configurable prop with a sensible default
Also applies to: 74-74, 99-99, 131-131, 152-152, 173-173, 200-200, 224-224, 248-248, 272-272, 303-303, 329-329, 351-351, 376-376, 415-415, 462-462, 495-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/src/content/docs/components/sidebar/demo.ts` at line 26, The demos currently add inline style style={{padding:"4px"}} to the Flex inside Sidebar.Header; replace this fragile pattern by moving the padding into a reusable component or stylesheet: either update the Sidebar.Header CSS (e.g., .header) to include the default content padding, or create a new Sidebar.HeaderContent component that wraps Flex and applies spacing via design tokens, or add a padding prop with a default on Sidebar.Header so callers no longer need inline styles; find occurrences using the Flex inside Sidebar.Header (and identifiers like Sidebar.Header, Flex) and refactor them to consume the shared padding behavior instead of inline style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/www/src/content/docs/components/sidebar/demo.ts`:
- Line 26: The demos currently add inline style style={{padding:"4px"}} to the
Flex inside Sidebar.Header; replace this fragile pattern by moving the padding
into a reusable component or stylesheet: either update the Sidebar.Header CSS
(e.g., .header) to include the default content padding, or create a new
Sidebar.HeaderContent component that wraps Flex and applies spacing via design
tokens, or add a padding prop with a default on Sidebar.Header so callers no
longer need inline styles; find occurrences using the Flex inside Sidebar.Header
(and identifiers like Sidebar.Header, Flex) and refactor them to consume the
shared padding behavior instead of inline style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be84353e-0909-4ece-81b5-ff0cca964067
📒 Files selected for processing (3)
apps/www/src/content/docs/components/sidebar/demo.tspackages/raystack/components/sidebar/sidebar-main.tsxpackages/raystack/components/sidebar/sidebar.module.css
paanSinghCoder
left a comment
paanSinghCoder
left a comment
There was a problem hiding this comment.
Not related to this PR - Lets add an example for simple Sidebar without Group.
2 participants
Summary
gap: var(--rs-space-6)on sidebar root and remove the gap from.mainso layout regions space themselves consistently..headerpadding to0 var(--rs-space-4)to align with the new root gap.SidebarMaintogap={3}so items inside main use 8px spacing.margin-top: var(--rs-space-4)(12px) to.nav-groupinside.main, skipping the first child so a leading group sits flush at the top.