Add thread drawer mode as alternative to tab layout#2150
Conversation
Implements a NavigationSplitView-based drawer for accessing chat history, with a toggle in Display settings to switch between tab and drawer modes. ## Key Features - New "Show thread list drawer" toggle in Settings > Display - Drawer mode shows threads in a collapsible sidebar (NavigationSplitView) - Thread hiding system: X button hides threads instead of deleting - Hidden threads shown in collapsible "HIDDEN" section at bottom - Restore button (↺) to unhide threads in drawer mode - Consistent button behavior across both modes (icon-only toolbar buttons) ## Implementation Details - ThreadModel: Added `isHidden` property to track hidden threads - ThreadManager: Modified `closeThread()` to mark threads as hidden - ThreadManager: Added `showThread()` to restore hidden threads - MainWindowView: Conditional layout based on `useThreadDrawer` setting - ChatView: Reduced top padding in drawer mode for better spacing - Both modes now filter hidden threads from main list ## Technical Choices - Reuses existing NavigationSplitView component (no custom drawer) - Leverages ThreadManager's existing thread state management - Minimal code duplication between tab and drawer modes - Uses SwiftUI's native collapsible Section for hidden threads Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
453cb71 to
8021ad3
Compare
- Sync selectedThreadId back from activeThreadId to keep sidebar selection in sync - Clean up ChatViewModels when threads are hidden to prevent memory leaks - Recreate ChatViewModels when hidden threads are restored - Use selectThread for visible threads instead of always calling showThread Addresses review feedback from devin-ai-integration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Resolved conflict in ChatView.swift by keeping both state variables: - useThreadDrawer (from feature branch) for drawer mode padding - composerScrollOffset (from main) for composer scroll handling Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1. Add deleteThread() method for permanent thread deletion - Allows users to clean up hidden threads in drawer mode - Prevents indefinite accumulation of hidden threads 2. Fix hidden thread row click behavior - Removed .tag() from hidden thread rows - Only explicit restore button triggers restoration now 3. Optimize duplicate filter computation - Extract hiddenThreads to local variable - Avoid computing filter twice 4. Add delete button to hidden threads UI - Trash icon alongside restore button - Provides permanent cleanup option Fixes issues #2, #3, and #4 from review feedback. Issue #1 (sessionId data loss) was already fixed in previous commit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1. ThreadModel.swift: Change sessionId from let to var - Fixes critical data loss bug where sessionId couldn't be persisted - Enables chat history restoration when threads are unhidden 2. VSplitView.swift: Improve split-view behavior - Change from overlay to side-by-side HStack layout - Panel now pushes content aside instead of covering it - Smoother spring animation (0.3s response, 0.8 damping) - Better padding (vertical + leading only) - Add clarifying comments These changes address the sessionId data loss issue (#1) and improve right panel UX per user feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Devin correctly identified that closeThread() now hides threads in both modes, but tab mode originally deleted them permanently. This is a behavioral regression. Fix: Make closeThread() conditional based on useThreadDrawer setting: - Tab mode (useThreadDrawer = false): Delete threads permanently (original) - Drawer mode (useThreadDrawer = true): Hide threads (new feature) This preserves the original tab behavior while enabling the new drawer restoration feature only where it's accessible via UI. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove all hidden thread functionality to match original behavior: - Removed isHidden field from ThreadModel - Removed showThread() and deleteThread() methods from ThreadManager - Restored original simple closeThread() that removes from memory - Removed HIDDEN section from drawer mode UI - Removed isHidden filters from thread lists Original behavior preserved: - X button removes thread from memory - Threads restored on app restart from daemon sessions (up to 5 most recent) - Both tab and drawer modes now have identical thread lifecycle This simplifies the implementation and matches the original behavior exactly. Hide/restore can be added as a separate feature later. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1. Disable toolbar animations during NavigationSplitView transitions - Add .transaction modifier to disable animations - Wrap columnVisibility changes in withTransaction - Prevents toolbar flash when toggling drawer 2. Hide X button on last thread in drawer mode - Conditionally show close button only when threads.count > 1 - Matches tab mode behavior (ThreadTabBar) - Prevents non-functional button clicks 3. Fix VSplitView trailing padding - Change from .padding(.leading) to .padding(.horizontal) - Ensures right-side rounded corners aren't clipped - Provides proper spacing on both sides Addresses Devin review issues from commit c4cb938. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed approach to fix toolbar flash: - Removed broad .transaction modifier that disabled all animations - Added .animation(nil, value: columnVisibility) to toolbar HStack only - NavigationSplitView sidebar now slides in/out smoothly - Toolbar no longer flashes during sidebar transitions This gives us the best of both worlds: smooth drawer animation and stable toolbar positioning. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The "extra button" flash was caused by NavigationSplitView automatically adding a native sidebar toggle button to the toolbar. When the sidebar opened/closed, macOS would reposition/show/hide this button, causing our toolbar buttons to shift and flash. Fix: Add .toolbar(removing: .sidebarToggle) to disable the automatic sidebar toggle button that NavigationSplitView adds by default. Also removed the .animation(nil) workaround since we're now fixing the root cause instead of hiding the symptom. This is the proper fix - no animation suppression needed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add panel styling to chat content (rounded corners, consistent background) - Remove top padding from panels so they extend to button bar - Add smooth spring animation to HStack to prevent jumping when drawer opens/closes - Match ThreadTabBar layout pattern with VStack wrapper Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Reopen thread drawer when closing right-side panel (fixes drawer staying hidden) - Revert VSplitView padding to exclude top padding (panels flush with button bar) Addresses Devin review feedback on PR #2150 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove duplicate .ignoresSafeArea(edges: .top) in drawer mode - Merge duplicate .onAppear blocks into single initialization - Add comment explaining 78pt padding for traffic light buttons - Change ThreadModel.sessionId back to let (no longer mutated) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Make chat panel styling conditional on drawer mode (fixes tab mode regression) - Revert VSplitView animation to VAnimation.standard (shared component fix) - Extract trafficLightPadding constant with detailed documentation Addresses critical review feedback: tab mode no longer gets unintended background/clipping, and VSplitView animation is consistent across all uses. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
When Settings (or any panel) was opened with drawer already closed, closing the panel would incorrectly reopen the drawer. Now tracks previous drawer state and only reopens if it was open before. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Drawer and panels now only open/close via manual toggle - no automatic closing of drawer when panel opens, no automatic reopening. Both are completely independent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Restructured threadItem to use Button-based approach like ThreadTab, with close button in overlay instead of nested in HStack. This prevents onTapGesture from intercepting close button clicks. Fixes Devin review feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
| isExpanded: $isDynamicExpanded, | ||
| daemonClient: daemonClient | ||
| ) | ||
| } else { |
There was a problem hiding this comment.
🟡 Right panel width calculated from full window width, not available space, causing unusable layout in drawer mode
When the thread drawer is open in drawer mode, opening a right-side panel (Settings, Debug, etc.) can squeeze the chat content to an unusable width because the panel width is calculated from the full window geometry rather than the space available to chatContentView.
Root Cause and Impact
The shared chatContentView(geometry:) at clients/macos/vellum-assistant/Features/MainWindow/MainWindowView.swift:238 calculates panel width as:
VSplitView(panelWidth: geometry.size.width / zoomManager.zoomLevel * 0.5, ...)The geometry comes from the outer GeometryReader (line 37) which measures the full window size. In tab mode this is correct because the full width is available. But in drawer mode, chatContentView is placed inside an HStack alongside the 240px-wide threadDrawerView (lines 82-96), so the available width is significantly reduced.
At the minimum window width of 800px (line 125: .frame(minWidth: 800, minHeight: 600)):
- Drawer: 240px +
VSpacing.smpadding ≈ 248px - Available for
chatContentView: ≈ 544px - Panel width: 800 × 0.5 = 400px
- Remaining for chat: ≈ 144px — effectively unusable
The auto-close-drawer-on-panel-open behavior mentioned in the PR description was explicitly removed in commit 3418358 ("Remove automatic drawer/panel linking behavior"), so both drawer and panel can now be open simultaneously with no mitigation for the cramped layout.
Impact: Users in drawer mode who open any side panel while the drawer is visible will see the chat content crushed to a narrow, unreadable strip, especially at smaller window sizes.
Prompt for agents
In MainWindowView.swift, the chatContentView function at line 238 calculates panelWidth as geometry.size.width / zoomManager.zoomLevel * 0.5, where geometry is the full window GeometryReader. In drawer mode, the chatContentView is placed in an HStack alongside the 240px drawer, so the available width is narrower. Fix by either:
1. Adjusting the panelWidth calculation to account for the drawer width when useThreadDrawer is true and the drawer is visible (columnVisibility != .detailOnly). For example, subtract the drawer width (240 + padding) from the geometry width before multiplying by 0.5.
2. Or, wrap the chatContentView in its own GeometryReader in drawer mode so the panel width is based on the actual available width.
3. Or, re-introduce the auto-close-drawer behavior when a panel opens (the approach that was removed in commit 3418358).
Option 1 is the most straightforward: change line 238 to something like:
let availableWidth = geometry.size.width / zoomManager.zoomLevel - (useThreadDrawer && columnVisibility != .detailOnly ? 256 : 0)
VSplitView(panelWidth: availableWidth * 0.5, showPanel: activePanel != nil, main: { ... }, panel: { ... })
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Adds an optional drawer-based navigation layout as an alternative to the traditional tab bar. Users can toggle between layouts in Settings. Thread lifecycle behavior is unchanged from the original implementation.
Key Features
Thread Drawer Toggle
@AppStorage("useThreadDrawer")Drawer Mode Layout
NavigationSplitViewwith collapsible sidebarTab Mode Layout (Original)
Right Panel Improvements
.id()to prevent flickerUI Consistency
Thread Lifecycle (Unchanged)
Both modes preserve the original thread behavior:
Implementation Details
Files Changed
closeThread()implementationselectThread()for programmatic selectionuseThreadDrawerselectedThreadIdandactiveThreadIdTechnical Choices
NavigationSplitViewprovides native drawer behavior (no custom implementation)ThreadManagerstate managementTesting
🤖 Generated with Claude Code