Skip to content

Fix buggy sidebar - #691

Merged
hanlun0804 merged 3 commits into
mainfrom
690-fix-buggy-sidebar
Aug 12, 2026
Merged

Fix buggy sidebar#691
hanlun0804 merged 3 commits into
mainfrom
690-fix-buggy-sidebar

Conversation

@hanlun0804

Copy link
Copy Markdown
Collaborator

No description provided.

@hanlun0804
hanlun0804 requested review from martinenilsen1 and a lite review from Copilot August 12, 2026 10:33
@hanlun0804 hanlun0804 linked an issue Aug 12, 2026 that may be closed by this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the shared Sidebar component implementation to improve behavior and maintainability, with a focus on clarifying component responsibilities and refining the collapsed-sidebar hover popover interactions.

Changes:

  • Added JSDoc-style documentation to key sidebar components/hooks to clarify intended usage and behavior.
  • Updated SidebarMenuButton hover behavior to keep the popover open briefly when moving the cursor from the button into the popover.
  • Minor styling tweaks (e.g., width/spacing utility adjustments) to align with the project’s Tailwind conventions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/web/src/components/shadcn/sidebar.tsx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

apps/web/src/components/shadcn/sidebar.tsx:570

  • {...props} is spread after onClick/onMouseEnter/onMouseLeave, so any consumer-provided handlers for these props will override the sidebar’s internal hover open/close behavior (and the current onClick wrapper is effectively bypassed if props.onClick exists). Consider spreading props first and wrapping handlers so internal logic always runs while still delegating to user handlers.
          cancelClose()
          if (state === 'collapsed') setOpen(true)
        }}
        onMouseLeave={scheduleClose}
        {...props}

@martinenilsen1 martinenilsen1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hanlun0804
hanlun0804 merged commit befae7d into main Aug 12, 2026
4 checks passed
@hanlun0804
hanlun0804 deleted the 690-fix-buggy-sidebar branch August 12, 2026 11:11
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.

Fix buggy sidebar

3 participants