-
Notifications
You must be signed in to change notification settings - Fork 0
fix(macos): resolve empty chat display during tab switching #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import React, { useState, useEffect, useRef, useMemo } from "react"; | ||
| import React, { useState, useEffect, useLayoutEffect, useRef, useMemo, useCallback } from "react"; | ||
| import { motion, AnimatePresence } from "framer-motion"; | ||
| import { | ||
| Copy, | ||
|
|
@@ -167,7 +167,7 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
|
|
||
| // Filter out messages that shouldn't be displayed | ||
| const displayableMessages = useMemo(() => { | ||
| return messages.filter((message, index) => { | ||
| const filtered = messages.filter((message, index) => { | ||
| // Skip meta messages that don't have meaningful content | ||
| if (message.isMeta && !message.leafUuid && !message.summary) { | ||
| return false; | ||
|
|
@@ -196,13 +196,13 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| for (let i = index - 1; i >= 0; i--) { | ||
| const prevMsg = messages[i]; | ||
| if (prevMsg.type === 'assistant' && prevMsg.message?.content && Array.isArray(prevMsg.message.content)) { | ||
| const toolUse = prevMsg.message.content.find((c: any) => | ||
| const toolUse = prevMsg.message.content.find((c: any) => | ||
| c.type === 'tool_use' && c.id === content.tool_use_id | ||
| ); | ||
| if (toolUse) { | ||
| const toolName = toolUse.name?.toLowerCase(); | ||
| const toolsWithWidgets = [ | ||
| 'task', 'edit', 'multiedit', 'todowrite', 'ls', 'read', | ||
| 'task', 'edit', 'multiedit', 'todowrite', 'ls', 'read', | ||
| 'glob', 'bash', 'write', 'grep' | ||
| ]; | ||
| if (toolsWithWidgets.includes(toolName) || toolUse.name?.startsWith('mcp__')) { | ||
|
|
@@ -226,13 +226,30 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| } | ||
| return true; | ||
| }); | ||
| }, [messages]); | ||
|
|
||
| // Log for debugging the empty chat issue on macOS | ||
| if (process.env.NODE_ENV === 'development') { | ||
| console.log('[ClaudeCodeSession] displayableMessages update:', { | ||
| total: messages.length, | ||
| displayable: filtered.length, | ||
| sessionId: session?.id | ||
| }); | ||
| } | ||
|
|
||
| return filtered; | ||
| }, [messages, session?.id]); | ||
|
|
||
| // Use stable virtualizer instance with proper key for remounting | ||
| const virtualizerKey = useMemo(() => { | ||
| return `virtualizer-${session?.id || 'new'}-${displayableMessages.length}`; | ||
| }, [session?.id, displayableMessages.length]); | ||
|
|
||
| const rowVirtualizer = useVirtualizer({ | ||
| count: displayableMessages.length, | ||
| getScrollElement: () => parentRef.current, | ||
| estimateSize: () => 150, // Estimate, will be dynamically measured | ||
| overscan: 5, | ||
| debug: false, | ||
| }); | ||
|
|
||
| // Debug logging | ||
|
|
@@ -247,12 +264,22 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| }); | ||
| }, [projectPath, session, extractedSessionInfo, effectiveSession, messages.length, isLoading]); | ||
|
|
||
| // Reset component state when session changes (critical for macOS tab switching) | ||
| useLayoutEffect(() => { | ||
| // Clear messages immediately when session changes to prevent race conditions | ||
| if (session) { | ||
| setMessages([]); | ||
| setError(null); | ||
| setIsLoading(true); | ||
| } | ||
| }, [session?.id]); | ||
|
|
||
| // Load session history if resuming | ||
| useEffect(() => { | ||
| if (session) { | ||
| // Set the claudeSessionId immediately when we have a session | ||
| setClaudeSessionId(session.id); | ||
|
|
||
| // Load session history first, then check for active session | ||
| const initializeSession = async () => { | ||
| await loadSessionHistory(); | ||
|
|
@@ -261,7 +288,7 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| await checkForActiveSession(); | ||
| } | ||
| }; | ||
|
|
||
| initializeSession(); | ||
| } | ||
| }, [session]); // Remove hasLoadedSession dependency to ensure it runs on mount | ||
|
|
@@ -271,13 +298,31 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| onStreamingChange?.(isLoading, claudeSessionId); | ||
| }, [isLoading, claudeSessionId, onStreamingChange]); | ||
|
|
||
| // Auto-scroll to bottom when new messages arrive | ||
| useEffect(() => { | ||
| if (displayableMessages.length > 0) { | ||
| rowVirtualizer.scrollToIndex(displayableMessages.length - 1, { align: 'end', behavior: 'smooth' }); | ||
| // Initialize virtual scroller properly after DOM updates | ||
| const scrollToBottom = useCallback(() => { | ||
| if (displayableMessages.length > 0 && rowVirtualizer) { | ||
| try { | ||
| rowVirtualizer.scrollToIndex(displayableMessages.length - 1, { align: 'end', behavior: 'smooth' }); | ||
| } catch (error) { | ||
| console.warn('Failed to scroll to index:', error); | ||
| // Fallback scroll to bottom | ||
| if (parentRef.current) { | ||
| parentRef.current.scrollTop = parentRef.current.scrollHeight; | ||
| } | ||
| } | ||
| } | ||
| }, [displayableMessages.length, rowVirtualizer]); | ||
|
|
||
| // Use layoutEffect for immediate DOM operations - better for macOS | ||
| useLayoutEffect(() => { | ||
| if (displayableMessages.length > 0) { | ||
| // Use requestAnimationFrame for smooth macOS rendering | ||
| requestAnimationFrame(() => { | ||
| scrollToBottom(); | ||
| }); | ||
| } | ||
| }, [displayableMessages.length, scrollToBottom]); | ||
|
|
||
| // Calculate total tokens from messages | ||
| useEffect(() => { | ||
| const tokens = messages.reduce((total, msg) => { | ||
|
|
@@ -323,12 +368,23 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| // After loading history, we're continuing a conversation | ||
| setIsFirstPrompt(false); | ||
|
|
||
| // Scroll to bottom after loading history | ||
| setTimeout(() => { | ||
| if (loadedMessages.length > 0) { | ||
| rowVirtualizer.scrollToIndex(loadedMessages.length - 1, { align: 'end', behavior: 'auto' }); | ||
| } | ||
| }, 100); | ||
| // Scroll to bottom after loading history with proper timing for macOS | ||
| // Use requestAnimationFrame for better macOS compatibility | ||
| requestAnimationFrame(() => { | ||
| setTimeout(() => { | ||
| if (loadedMessages.length > 0) { | ||
| try { | ||
| rowVirtualizer.scrollToIndex(loadedMessages.length - 1, { align: 'end', behavior: 'auto' }); | ||
| } catch (error) { | ||
| console.warn('Failed to scroll to index after loading history:', error); | ||
| // Fallback scroll | ||
| if (parentRef.current) { | ||
| parentRef.current.scrollTop = parentRef.current.scrollHeight; | ||
| } | ||
| } | ||
| } | ||
| }, 100); // Reduced delay since we're using RAF | ||
| }); | ||
| } catch (err) { | ||
| console.error("Failed to load session history:", err); | ||
| setError("Failed to load session history"); | ||
|
|
@@ -1145,6 +1201,7 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| style={{ | ||
| contain: 'strict', | ||
| }} | ||
| key={virtualizerKey} // Force re-render when virtualizer key changes | ||
| > | ||
| <div | ||
| className="relative w-full max-w-6xl mx-auto px-4 pt-8 pb-4" | ||
|
|
@@ -1154,30 +1211,42 @@ export const ClaudeCodeSession: React.FC<ClaudeCodeSessionProps> = ({ | |
| }} | ||
| > | ||
| <AnimatePresence> | ||
| {rowVirtualizer.getVirtualItems().map((virtualItem) => { | ||
| const message = displayableMessages[virtualItem.index]; | ||
| return ( | ||
| <motion.div | ||
| key={virtualItem.key} | ||
| data-index={virtualItem.index} | ||
| ref={(el) => el && rowVirtualizer.measureElement(el)} | ||
| initial={{ opacity: 0, y: 8 }} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| exit={{ opacity: 0, y: -8 }} | ||
| transition={{ duration: 0.3 }} | ||
| className="absolute inset-x-4 pb-4" | ||
| style={{ | ||
| top: virtualItem.start, | ||
| }} | ||
| > | ||
| <StreamMessage | ||
| message={message} | ||
| streamMessages={messages} | ||
| onLinkDetected={handleLinkDetected} | ||
| /> | ||
| </motion.div> | ||
| ); | ||
| })} | ||
| {rowVirtualizer && displayableMessages.length > 0 ? ( | ||
| rowVirtualizer.getVirtualItems().map((virtualItem) => { | ||
| const message = displayableMessages[virtualItem.index]; | ||
| if (!message) return null; // Guard against undefined messages | ||
|
|
||
| return ( | ||
| <motion.div | ||
| key={`${virtualizerKey}-${virtualItem.key}`} // Include virtualizer key for better React reconciliation | ||
| data-index={virtualItem.index} | ||
| ref={(el) => el && rowVirtualizer.measureElement(el)} | ||
| initial={{ opacity: 0, y: 8 }} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| exit={{ opacity: 0, y: -8 }} | ||
| transition={{ duration: 0.3 }} | ||
| className="absolute inset-x-4 pb-4" | ||
| style={{ | ||
| top: virtualItem.start, | ||
| }} | ||
| > | ||
| <StreamMessage | ||
| message={message} | ||
| streamMessages={messages} | ||
| onLinkDetected={handleLinkDetected} | ||
| /> | ||
| </motion.div> | ||
| ); | ||
| }) | ||
| ) : ( | ||
| // Show empty state when no messages to help debug | ||
| displayableMessages.length === 0 && messages.length > 0 && ( | ||
| <div className="absolute inset-x-4 py-8 text-center text-muted-foreground"> | ||
| <p>Messages are being processed...</p> | ||
| <p className="text-xs mt-2">Raw: {messages.length}, Filtered: {displayableMessages.length}</p> | ||
| </div> | ||
| ) | ||
|
Comment on lines
+1243
to
+1248
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This "Messages are being processed..." state is a good idea for providing feedback instead of showing a blank screen. However, it currently includes debugging information ( I suggest two small improvements:
|
||
| )} | ||
| </AnimatePresence> | ||
| </div> | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of
requestAnimationFrameandtry-catchhere is a great improvement for robustness. However, thesetTimeoutwith a 100ms delay introduces a magic number and can be brittle—potentially too long on fast devices and too short on slow ones.If the goal is to wait for the next render cycle after
requestAnimationFrame, asetTimeoutwith a0delay is usually sufficient and more idiomatic. This pushes the execution to the end of the browser's event queue, ensuring it runs after the current execution context (including rendering) is complete.Consider using a
0delay unless the 100ms wait is strictly necessary. If it is, it would be helpful to add a comment explaining why.