-
Notifications
You must be signed in to change notification settings - Fork 17
fix: sidebar toggle behavior and layout #274
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: main
Are you sure you want to change the base?
fix: sidebar toggle behavior and layout #274
Conversation
|
All contributors have signed the CLA. Thank you! |
Greptile SummaryReplaces hover-based sidebar expansion with click-based toggle behavior. The sidebar now toggles via clicking the logo icon or empty space (excluding interactive elements marked with
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant SidebarLayout
participant AppSidebar
participant SidebarBranding
participant localStorage
Note over SidebarLayout: On mount
SidebarLayout->>localStorage: getItem('browseros-sidebar-expanded')
localStorage-->>SidebarLayout: stored value or null
SidebarLayout->>SidebarLayout: setSidebarOpen(stored === 'true')
Note over User,SidebarBranding: Toggle via logo click
User->>SidebarBranding: Click logo button
SidebarBranding->>SidebarBranding: event.stopPropagation()
SidebarBranding->>SidebarLayout: onToggle()
SidebarLayout->>SidebarLayout: toggleSidebar()
SidebarLayout->>localStorage: setItem('browseros-sidebar-expanded', String(!prev))
SidebarLayout->>AppSidebar: expanded={newValue}
Note over User,AppSidebar: Toggle via empty space click
User->>AppSidebar: Click empty space
AppSidebar->>AppSidebar: handleSidebarClick(event)
AppSidebar->>AppSidebar: Check if target.closest('[data-sidebar-interactive]')
AppSidebar->>SidebarLayout: onToggle()
SidebarLayout->>SidebarLayout: toggleSidebar()
SidebarLayout->>localStorage: setItem('browseros-sidebar-expanded', String(!prev))
SidebarLayout->>AppSidebar: expanded={newValue}
Note over User,SidebarBranding: Click on interactive element
User->>SidebarBranding: Click ThemeToggle (data-sidebar-interactive)
AppSidebar->>AppSidebar: handleSidebarClick detects interactive element
AppSidebar->>AppSidebar: return early (no toggle)
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
I have read the CLA Document and I hereby sign the CLA |
|
Hey, thanks for the contribution. But we want to keep the hover based interaction. That was an intended feature. What was the thought process in changing to click based? |
Summary
Testing
Manual: verified sidebar toggle, visibility, and layout shift
Screenshots
Before:
After: