fix: add collapsible functionality to ReasoningBlock for better performance (#8066) #8068
Conversation
…rmance - Added expand/collapse functionality to ReasoningBlock component - Integrated with existing ChatRow expand/collapse state management - Uses ToolUseBlock for consistent UI styling - Improves performance by reducing DOM rendering for large thinking sections - Fixes issue #8066 where reasoning sections caused sluggish UI performance
dosubot
bot
added
size:M
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| </div> | ||
| </ToolUseBlock> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Missing test coverage for this component. Since this is a critical performance fix, would it be worth adding tests to ensure the collapsible functionality works correctly and prevents regression?
| <ToolUseBlock> | ||
| <div | ||
| className="flex items-center justify-between px-3 py-2 cursor-pointer hover:bg-vscode-list-hoverBackground" | ||
| onClick={toggleExpand}> |
There was a problem hiding this comment.
Could we improve accessibility here? The clickable div doesn't have keyboard navigation support. Consider adding tabIndex={0}, role="button", aria-expanded={isExpanded}, and keyboard event handlers for Enter/Space keys.
| const [elapsed, setElapsed] = useState<number>(0) | ||
|
|
||
| // Use internal state if no external control is provided | ||
| const [internalIsExpanded, setInternalIsExpanded] = useState(false) |
There was a problem hiding this comment.
Given the performance issues mentioned in #8066, should we consider starting with collapsed state by default? Currently it starts expanded (false becomes expanded) when using internal state. This might provide better initial performance, especially for large thinking sections.
| </span> | ||
| )} | ||
| {hasContent && | ||
| (isExpanded ? <ChevronUp className="w-4 h-4" /> : <ChevronDown className="w-4 h-4" />)} |
There was a problem hiding this comment.
Consider adding aria-labels for better screen reader support on the chevron icons. For example: aria-label="Collapse" for ChevronUp and aria-label="Expand" for ChevronDown.
| {(content?.trim()?.length ?? 0) > 0 && ( | ||
| <div className="px-3 italic text-vscode-descriptionForeground"> | ||
| {hasContent && isExpanded && ( | ||
| <div className="px-4 py-2 italic text-vscode-descriptionForeground border-t border-vscode-panel-border"> |
There was a problem hiding this comment.
Is the nested padding intentional? The ToolUseBlock wrapper adds p-2, and then this content area has px-4 py-2. This might create inconsistent spacing compared to other tool blocks. Consider aligning the padding strategy.
Description
This PR addresses Issue #8066 by adding collapsible functionality to the ReasoningBlock component, which resolves the performance issues users were experiencing with reasoning model thinking sections.
Problem
Solution
Changes
Testing
Performance Impact
The collapsible functionality should significantly improve UI responsiveness by:
Fixes #8066
Important
Adds collapsible functionality to
ReasoningBlockfor improved performance by reducing DOM nodes when collapsed, integrated withChatRowstate management.ReasoningBlockinReasoningBlock.tsx.ChatRowinChatRow.tsxfor expand/collapse state management.ToolUseBlockfor consistent styling.This description was created by
for 66598d5. You can customize this summary. It will automatically update as commits are pushed.