Repository navigation
fix(ui): fix Activity and History buttons not opening sheet on mobile - #273
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughMigrates mobile sheet open state from local component state to the centralized Zustand layout store by adding Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant GAV as GlobalAssistantView
participant Store as useLayoutStore
participant Layout as Layout (Sheet UI)
User->>GAV: clicks "Open Activity"/"Open History"
GAV->>Store: setRightSidebarOpen(true)
GAV->>Store: setRightSheetOpen(true)
Store-->>Layout: rightSidebarOpen/rightSheetOpen updated
Layout->>Layout: render right sheet (close left if open)
Layout-->>User: right sheet visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
124f10e to
56e78ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/layout/Layout.tsx (2)
98-170: Add missing sidebar setters to dependency arrays in these callbacks.
handleLeftPanelToggleusessetRightSidebarOpenon line 116 but doesn't include it in the dependency array. Similarly,handleRightPanelToggleusessetLeftSidebarOpenon line 159 but omits it from its dependency array. This will triggerreact-hooks/exhaustive-depsand risks stale closures.Add missing dependencies
}, [ isSheetBreakpoint, leftSheetOpen, rightSheetOpen, shouldOverlaySidebars, leftSidebarOpen, rightSidebarOpen, setLeftSheetOpen, setRightSheetOpen, setLeftSidebarOpen, + setRightSidebarOpen, toggleLeftSidebar, ]); ... }, [ isSheetBreakpoint, leftSheetOpen, rightSheetOpen, shouldOverlaySidebars, leftSidebarOpen, rightSidebarOpen, setLeftSheetOpen, setRightSheetOpen, + setLeftSidebarOpen, setRightSidebarOpen, toggleRightSidebar, ]);
74-88: Add store setters to these effect dependency arrays.These effects call
setLeftSheetOpenandsetRightSheetOpenfrom Zustand. Since the ESLint config enablesreact-hooks/exhaustive-deps(vianext/core-web-vitals), the rule will flag these selector-based setters as missing dependencies. Include them in both dependency arrays:Proposed fix
- }, [isSheetBreakpoint]); + }, [isSheetBreakpoint, setLeftSheetOpen, setRightSheetOpen]); ... - }, [pathname, isSheetBreakpoint]); + }, [pathname, isSheetBreakpoint, setLeftSheetOpen, setRightSheetOpen]);
The Activity and History buttons in GlobalAssistantView were only toggling the rightSidebarOpen state, which controls the desktop sidebar. On mobile, the sidebar is rendered as a Sheet component with separate state. Changes: - Add leftSheetOpen and rightSheetOpen state to useLayoutStore (not persisted) - Update Layout.tsx to use sheet state from store instead of local useState - Update handleOpenActivity and handleOpenHistory to set both sidebar and sheet open states, ensuring visibility on all breakpoints https://claude.ai/code/session_01TD1VRYC1c8UtJFMXXcvGYD
56e78ca to
8f508ee
Compare
The Activity and History buttons in GlobalAssistantView were only toggling
the rightSidebarOpen state, which controls the desktop sidebar. On mobile,
the sidebar is rendered as a Sheet component with separate state.
Changes:
sheet open states, ensuring visibility on all breakpoints
https://claude.ai/code/session_01TD1VRYC1c8UtJFMXXcvGYD
Summary by CodeRabbit
Refactor
Bug Fixes
Chore
✏️ Tip: You can customize this high-level summary in your review settings.