feat: add sidebar pinning functionality in offscreen mode #123
Conversation
- Introduce pinning capability for the sidebar in offscreen mode, allowing users to keep it open regardless of hover state. - Update sidebar behavior to prevent auto-hiding when pinned. - Refactor sidebar context to manage pinned state and variant. - Enhance navigation controls to include a pin button for user interaction.
WalkthroughThe changes introduce a "pinned" state to the sidebar, allowing users to pin or unpin it. This state is managed via context, persisted in cookies, and exposed through a new pin button in the sidebar header. Sidebar hover and auto-hide behaviors are updated to respect the pinned state, and related imports are reorganized for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PinButton
participant SidebarContext
participant Sidebar
participant Cookie
User->>PinButton: Click pin/unpin
PinButton->>SidebarContext: togglePin()
SidebarContext->>Sidebar: Update pinned state
SidebarContext->>Cookie: Persist pinned state
Sidebar->>Sidebar: Render with data-pinned attribute
PinButton->>PinButton: Update icon and tooltip
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a sidebar pinning functionality in offscreen mode by updating the sidebar’s behavior and context state management along with UI controls for pinning.
- Added pinned state management with cookie persistence.
- Integrated UI changes including a new PinButton component and conditional auto-hide behavior.
- Refactored sidebar context and hover detection logic to incorporate the pinned state.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/renderer/src/components/ui/resizable-sidebar.tsx | Added pinned state management, cookie persistence, and updated context. |
| src/renderer/src/components/browser-ui/sidebar/header/action-buttons.tsx | Added PinButton and updated sidebar interaction logic. |
| src/renderer/src/components/browser-ui/main.tsx | Modified auto-hide logic for the floating sidebar based on pinned state. |
| src/renderer/src/components/browser-ui/browser-sidebar.tsx | Updated hover detection to account for the pinned state. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/renderer/src/components/browser-ui/main.tsx (1)
52-57: Avoid unnecessary re-creation of the auto-hide effect
setOpenis a stable function coming fromuseSidebar; keeping it in the dependency array needlessly re-creates this effect every render and can cause “exhaustive-deps” noise in linters.
The dependency can safely be removed.-}, [isHoveringSidebar, open, variant, pinned, setOpen]); +}, [isHoveringSidebar, open, variant, pinned]);src/renderer/src/components/browser-ui/browser-sidebar.tsx (1)
60-90: Clean up pending hover-state timeouts on unmount
useSidebarHoversetstimeoutIdRefbut never clears it when the component unmounts.
In practice that’s a memory leak risk and can fire state updates on an unmounted component.function useSidebarHover(setIsHoveringSidebar: (isHovering: boolean) => void) { const { pinned } = useSidebar(); const isHoveringSidebarRef = useRef(false); const timeoutIdRef = useRef<NodeJS.Timeout | null>(null); + // Clear any outstanding timers on unmount + useEffect(() => () => { + if (timeoutIdRef.current) { + clearTimeout(timeoutIdRef.current); + } + }, []);src/renderer/src/components/browser-ui/sidebar/header/action-buttons.tsx (1)
97-108: Add accessible label for the pin toggleThe
PinButtonrelies on thetitleattribute, which is not read by most screen readers.
Provide an explicitaria-labelto keep the control accessible.- /> + aria-label={pinned ? "Unpin sidebar" : "Pin sidebar"} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/renderer/src/components/browser-ui/browser-sidebar.tsx(4 hunks)src/renderer/src/components/browser-ui/main.tsx(2 hunks)src/renderer/src/components/browser-ui/sidebar/header/action-buttons.tsx(3 hunks)src/renderer/src/components/ui/resizable-sidebar.tsx(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/renderer/src/components/browser-ui/main.tsx (1)
src/renderer/src/components/ui/resizable-sidebar.tsx (1)
useSidebar(787-787)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Seems like an extra button that isn't really needed. How useful would this be? |
Sorry for getting back this late. I haven't been able to keep up with the latest developments on this awesome project. |
2 participants
SS:
Summary by CodeRabbit
New Features
Improvements