-
Notifications
You must be signed in to change notification settings - Fork 30
refactor(ui): implement collapsible sidebar with smooth animations and polish #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe PR introduces a collapsible sidebar navigation system with Zustand state management, relocates branding to the sidebar header, minimizes the title bar, and applies comprehensive UI styling enhancements including rounded corners, backdrop blur, transitions, and hover effects across components. Changes
Sequence DiagramsequenceDiagram
actor User
participant Nav as Nav Component
participant Store as Sidebar Store
participant App as App Component
participant UI as UI Render
User->>Nav: Click collapse/expand toggle
Nav->>Store: Call toggle()
Store->>Store: Update isCollapsed state
Store->>Nav: Notify subscribers
Store->>App: Notify subscribers
par Parallel Re-render
Nav->>Nav: Apply dynamic width (w-14 or w-60)
Nav->>Nav: Toggle label visibility
Nav->>UI: Render collapsed/expanded sidebar
App->>App: Apply dynamic margin (ml-14 or ml-60)
App->>UI: Render layout with adjusted spacing
end
UI->>User: Display updated layout with animation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/renderer/src/components/titlebar.jsx (1)
3-3: Remove unused import.The
sparkleLogoimport is no longer used since the logo was moved to the Sidebar. This is dead code.🔎 Proposed fix
import { Minus, Square, X } from "lucide-react" import { close, minimize, toggleMaximize } from "../lib/electron" -import sparkleLogo from "../../../../resources/sparklelogo.png"src/renderer/src/components/ui/dropdown.jsx (2)
25-29: Consider using a ref instead of class selector for portal detection.The click-outside handler relies on
.dropdown-portalclass which creates a coupling between the handler logic and the portal's CSS class. If the class is changed or removed, clicks inside the portal would incorrectly close the dropdown.A more robust approach would be to use a ref for the portal container.
🔎 Proposed fix using a portal ref
export function Dropdown({ options, value, onChange }) { const [isOpen, setIsOpen] = useState(false) const [coords, setCoords] = useState({ top: 0, left: 0, width: 0 }) const dropdownRef = useRef(null) + const portalRef = useRef(null) // ... resize/scroll effect ... useEffect(() => { function handleClickOutside(event) { if ( dropdownRef.current && !dropdownRef.current.contains(event.target) && - !event.target.closest(".dropdown-portal") + !(portalRef.current && portalRef.current.contains(event.target)) ) { setIsOpen(false) } } document.addEventListener("mousedown", handleClickOutside) return () => document.removeEventListener("mousedown", handleClickOutside) }, [])Then assign
ref={portalRef}to the portal container div.
61-88: Consider viewport boundary handling for edge cases.The portal positioning uses fixed coordinates without checking if the dropdown would render off-screen. On smaller screens or when the trigger is near the bottom of the viewport, the dropdown could be cut off.
This is a minor UX concern and may not be an issue for your current use cases.
src/renderer/src/pages/Home.jsx (1)
217-217: Consider extracting repeated card styling to avoid duplication.This Card instance overrides multiple styles that partially overlap with the base Card component (which already includes
rounded-3xl,hover:border-sparkle-primary/50, andbackdrop-blur-sm). The overrides here usebackdrop-blur-md(vsblur-smin base) and addbg-sparkle-card/60.Consider either:
- Creating a variant prop on Card for this translucent style, or
- Extracting a reusable class/component since InfoCard (Line 17 in
infocard.jsx) uses similar styling.src/renderer/src/components/ui/card.jsx (1)
3-15: Note:hover:-translate-y-1may cause layout shifts in dense grids.The lift animation on hover is a nice touch, but applying it globally to all Card instances may cause unintended layout shifts in tightly packed grids (like the Tweaks page with
h-44cards). Cards lifting up could overlap with cards above them.If this becomes an issue, consider making the lift effect opt-in via a prop or disabling it for specific use cases.
src/renderer/src/components/nav.jsx (1)
102-103: Clarify the emptyvariant=""intent.The Button component's
variantsobject doesn't include an empty string key, sovariant=""results in no variant-specific classes being applied—only the base styles. If this is intentional (to create a "ghost" button style), consider adding an explicitghostorunstyledvariant for clarity.🔎 Suggested improvement
- variant="" + variant="ghost"And in
button.jsx, add a ghost variant:const variants = { // ... existing variants ghost: "hover:bg-sparkle-card/50", }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
src/renderer/src/App.csssrc/renderer/src/App.jsxsrc/renderer/src/components/greeting.jsxsrc/renderer/src/components/infocard.jsxsrc/renderer/src/components/nav.jsxsrc/renderer/src/components/titlebar.jsxsrc/renderer/src/components/ui/button.jsxsrc/renderer/src/components/ui/card.jsxsrc/renderer/src/components/ui/dropdown.jsxsrc/renderer/src/components/ui/input.jsxsrc/renderer/src/pages/Home.jsxsrc/renderer/src/pages/Settings.jsxsrc/renderer/src/pages/Tweaks.jsxsrc/renderer/src/store/sidebarStore.jstweaks/registry-scripts.jsontweaks/registry.json
🧰 Additional context used
🧬 Code graph analysis (5)
src/renderer/src/App.jsx (3)
src/renderer/src/pages/Settings.jsx (1)
theme(21-21)src/renderer/src/components/nav.jsx (1)
useSidebarStore(60-60)src/renderer/src/store/sidebarStore.js (1)
useSidebarStore(4-15)
src/renderer/src/components/titlebar.jsx (1)
src/renderer/src/lib/electron.js (3)
minimize(1-3)toggleMaximize(5-7)close(9-11)
src/renderer/src/pages/Settings.jsx (1)
src/renderer/src/App.jsx (1)
theme(22-22)
src/renderer/src/components/nav.jsx (3)
src/renderer/src/App.jsx (1)
useSidebarStore(23-23)src/renderer/src/store/sidebarStore.js (1)
useSidebarStore(4-15)src/renderer/src/components/ui/button.jsx (1)
Button(11-52)
src/renderer/src/store/sidebarStore.js (2)
src/renderer/src/components/nav.jsx (1)
useSidebarStore(60-60)src/renderer/src/App.jsx (1)
useSidebarStore(23-23)
🔇 Additional comments (29)
src/renderer/src/components/greeting.jsx (1)
51-51: LGTM!The font size increase from
text-2xltotext-3xlwithmb-5spacing aligns well with the broader UI polish objectives of this PR.tweaks/registry.json (1)
2-2: LGTM!Version timestamp update to track registry changes.
tweaks/registry-scripts.json (2)
2-2: LGTM!Version timestamp synchronized with
registry.json.
52-52: LGTM!The debloat-windows script content appears unchanged; only formatting adjustments were made.
src/renderer/src/components/ui/button.jsx (1)
20-21: LGTM!Good styling enhancements:
focus-visible:ring-*classes improve keyboard accessibilityactive:scale-90provides satisfying interaction feedback- Shadow effects add depth consistent with the UI polish goals
src/renderer/src/pages/Settings.jsx (4)
164-165: LGTM!The
rounded-fullstyling creates pill-shaped theme selectors, consistent with the rounded design language applied across other UI components in this PR.
201-204: LGTM!Formatting adjustment for the conditional className; no behavioral change.
275-278: LGTM!Consistent formatting pattern with other status badge spans.
348-351: LGTM!Formatting consistent with other status badges in this file.
src/renderer/src/components/ui/input.jsx (2)
11-12: LGTM!Good enhancements:
rounded-fullmaintains design consistency with other componentsbackdrop-blur-smand translucent background add depthfocus:ring-2improves accessibility for keyboard users
25-27: LGTM!The
LargeInputstyling mirrors theInputcomponent's enhancements, maintaining visual consistency. Thefocus-withinring properly highlights the container when the inner input is focused.src/renderer/src/components/titlebar.jsx (1)
9-33: LGTM!The compact titlebar design (h-8, transparent background, smaller icons) works well with the new sidebar-based layout. The button dimensions and icon sizes are appropriately scaled.
src/renderer/src/components/ui/dropdown.jsx (3)
3-3: LGTM!Using
createPortalfor the dropdown panel is a good approach to avoid z-index and overflow clipping issues.
11-21: LGTM!Closing the dropdown on resize/scroll prevents stale positioning and ensures good UX. Using capture phase (
true) for scroll events catches scrolls on nested containers.
37-47: LGTM!The position calculation using
getBoundingClientRect()is correct. Calculating coords only when opening (not when already open) is efficient.src/renderer/src/pages/Tweaks.jsx (2)
394-397: LGTM!The
rounded-fullstyling for category filter buttons is consistent with the broader UI polish across the PR.
412-413: Verify content fits in reduced card height.The card height was reduced from
h-52toh-44and padding fromp-5top-4. This might cause content overflow for tweaks with longer descriptions. The overflow is partially handled by theoverflow-y-autoon Line 546, but ensure the reduced height provides adequate space for typical content.src/renderer/src/store/sidebarStore.js (1)
1-17: LGTM!Clean Zustand store implementation with persistence. The store correctly exposes:
isCollapsedstate for sidebar visibilitytoggle()for toggling statesetCollapsed()for explicit state settingThe persist middleware configuration is appropriate for localStorage-based persistence.
src/renderer/src/pages/Home.jsx (1)
144-144: LGTM!Minor grid gap adjustment from 4 to 3 for tighter spacing.
src/renderer/src/components/infocard.jsx (1)
17-22: LGTM!The updated styling with translucent backgrounds, backdrop blur, and refined hover states aligns well with the broader UI polish in this PR. The reduced margin (
mb-2) tightens the layout appropriately.src/renderer/src/App.jsx (2)
18-23: LGTM!Clean integration of the sidebar store. The
isCollapsedstate is properly consumed to drive the dynamic layout.
71-77: LGTM!The dynamic margin classes (
ml-14/ml-60) correctly correspond to the sidebar widths (w-14/w-60) innav.jsx. The transition timing (duration-500,cubic-bezier(0.2,0,0,1)) matches the sidebar's animation, ensuring synchronized movement.src/renderer/src/App.css (2)
1-1: LGTM!Clean consolidation of the font import with
layer(base).
146-155: LGTM!The Toastify style overrides use
!importantappropriately to override the library's default styles and integrate with the custom theme system.src/renderer/src/components/nav.jsx (5)
24-27: LGTM!Clean imports for the new sidebar functionality, including the store, icons, and logo asset.
72-86: Well-implemented accessible toggle button.Good implementation details:
- Proper
aria-labelandaria-expandedfor screen readersfocus-visiblering for keyboard navigation- Hover-only visibility with
opacity-0 group-hover:opacity-100while maintaining focus accessibilitypointer-events-nonewith conditional enable prevents accidental clicks when hidden
87-99: LGTM!The logo/branding section animates smoothly with the collapse state. The
max-w-0/max-w-[200px]technique with overflow hidden creates a clean width transition.
117-125: LGTM!The tab labels and "New" badge handle the collapsed state well:
- Labels use
max-w-0/max-w-[150px]for smooth width transitions- The "utilities" tab shows a dot indicator when collapsed (Line 124) as an alternative to the full badge
167-175: LGTM!The social icons section correctly switches between
flex-rowandflex-colbased on the collapsed state, maintaining usability in both layouts.
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
|
@coderabbitai generate unit tests |
Docstrings generation was requested by @borhanidev. * #56 (comment) The following files were modified: * `src/renderer/src/App.jsx` * `src/renderer/src/components/greeting.jsx` * `src/renderer/src/components/nav.jsx` * `src/renderer/src/components/titlebar.jsx` * `src/renderer/src/components/ui/card.jsx` * `src/renderer/src/components/ui/dropdown.jsx` * `src/renderer/src/components/ui/input.jsx` * `src/renderer/src/pages/Home.jsx` * `src/renderer/src/pages/Settings.jsx` * `src/renderer/src/pages/Tweaks.jsx`
|
@coderabbitai generate unit tests |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Unit Test Generation process completed. You can view the generated tests in this PR: View PR |
✅ Actions performedUnit Test Generation started... |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Unit Test Generation process completed. You can view the generated tests in this PR: View PR |
✅ Actions performedUnit Test Generation started... |
Refines Sidebar UI and Logic:
Summary by CodeRabbit
New Features
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.