chore(studio): Coding agent adjust layout - #676
Conversation
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
|
📝 WalkthroughWalkthroughGlobalNav now applies route-specific sidebar defaults and backgrounds. Claude Code history and skills use independently persisted floating panels. Artifact rendering is conditional and simplified, while selection labeling avoids treating redeployment actions as agent names. ChangesGlobal navigation behavior
Claude Code panel experience
Sequence Diagram(s)sequenceDiagram
participant Router
participant GlobalNav
participant SidebarState
Router->>GlobalNav: provide current route
GlobalNav->>SidebarState: apply route-specific default
SidebarState-->>GlobalNav: return persisted state
GlobalNav-->>Router: render updated navigation
sequenceDiagram
participant User
participant ClaudeCodeHistoryPanel
participant FloatingPanel
participant LocalStorage
User->>ClaudeCodeHistoryPanel: click expand or collapse
ClaudeCodeHistoryPanel->>FloatingPanel: update panel state
FloatingPanel-->>ClaudeCodeHistoryPanel: render expanded content
ClaudeCodeHistoryPanel->>LocalStorage: persist open panel
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/packages/studio/src/util/hooks/useSidebarState.ts (1)
16-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
defaultExpandedonly affects the initial mount, not later renders.
useLocalStorage's value is set via a lazyuseStateinitializer, sodefaultExpandedis only consulted once per mount. Callers changing this argument without remounting (GlobalNav currently forces this via akey) will see no effect. Worth a short comment/JSDoc noting this constraint so future call sites don't silently break.📝 Suggested doc note
+/** + * `defaultExpanded` is only used to seed the initial persisted value on mount. + * Changing it on a subsequent render has no effect unless the consuming + * component is remounted (e.g. via a `key` change). + */ export const useSidebarState = (defaultExpanded = true) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/util/hooks/useSidebarState.ts` around lines 16 - 20, Add a concise JSDoc or inline comment above useSidebarState documenting that defaultExpanded is only used during the initial mount because useLocalStorage lazily initializes its state, and changes to the argument require remounting to take effect. Do not alter the hook behavior.web/packages/studio/src/components/Layouts/GlobalNav/index.tsx (1)
108-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffRoute-keyed remount resets the whole nav subtree, not just the sidebar.
Keying
GlobalNavContenton route category forces AppBar, Breadcrumbs, ThemeSwitch, UserPopover, and ClaudeCodeTopBarChat to unmount/remount too, dropping any transient UI state (e.g. an open popover) whenever crossing into/out of the Code Agent route. Consider scoping the reset touseSidebarStateitself (react todefaultExpandedchanges there) instead of remounting the whole content tree.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/components/Layouts/GlobalNav/index.tsx` around lines 108 - 124, The key on GlobalNavContent causes the entire navigation subtree to remount when switching route categories. Remove the route-based key from GlobalNav and update useSidebarState so it reacts to defaultExpanded changes, resetting only sidebar state while preserving AppBar, Breadcrumbs, ThemeSwitch, UserPopover, and ClaudeCodeTopBarChat state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/Layouts/GlobalNav/index.tsx`:
- Line 99: Update the conditional class in the GlobalNav layout’s sidebar
className so the vertical navigation background utility uses Tailwind v4 syntax,
changing the important modifier from the v3 prefix form to the suffix form while
preserving the existing selector and route condition.
---
Nitpick comments:
In `@web/packages/studio/src/components/Layouts/GlobalNav/index.tsx`:
- Around line 108-124: The key on GlobalNavContent causes the entire navigation
subtree to remount when switching route categories. Remove the route-based key
from GlobalNav and update useSidebarState so it reacts to defaultExpanded
changes, resetting only sidebar state while preserving AppBar, Breadcrumbs,
ThemeSwitch, UserPopover, and ClaudeCodeTopBarChat state.
In `@web/packages/studio/src/util/hooks/useSidebarState.ts`:
- Around line 16-20: Add a concise JSDoc or inline comment above useSidebarState
documenting that defaultExpanded is only used during the initial mount because
useLocalStorage lazily initializes its state, and changes to the argument
require remounting to take effect. Do not alter the hook behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d05ea320-c10d-4ee4-868c-7c86f1dd4d1c
📒 Files selected for processing (15)
web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsxweb/packages/studio/src/components/Layouts/GlobalNav/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeLayout.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/artifacts.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/artifacts.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/ArtifactSections.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/ClaudeCodeArtifactsPane.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/FloatingPanel.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/constants.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/helpers.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/types.tsweb/packages/studio/src/util/hooks/useSidebarState.tsweb/packages/studio/src/util/localStorage.ts
💤 Files with no reviewable changes (3)
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/constants.tsx
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/types.ts
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/helpers.ts
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/Layouts/GlobalNav/index.tsx`:
- Line 99: Update the NavigationDrawer className to always include
sidebarBackground, while keeping only the nested
[&_.nv-vertical-nav-root]:bg-transparent! override conditional on
isClaudeCodeChatRoute. Preserve the existing sizing, overflow, and grid-area
classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 55a3f76a-d84f-4719-b80c-a5bf7b696869
📒 Files selected for processing (1)
web/packages/studio/src/components/Layouts/GlobalNav/index.tsx
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsx`:
- Around line 238-258: Update the `ClaudeCodeHistoryPanel` test fixture so
`selections` is an actually empty array, keeping the existing assertions for
omitted sections and dividers. If whitespace-only selection values are expected
to be ignored, cover that behavior in a separate test rather than combining it
with the empty-section case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6a6fab09-120e-4859-af4d-14187653a9f3
📒 Files selected for processing (2)
web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/ClaudeCodeArtifactsPane.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/ClaudeCodeArtifactsPane.tsx
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Summary by CodeRabbit