feat: add autoExpandDiffs setting to auto-expand diffs in chat messages#11316
feat: add autoExpandDiffs setting to auto-expand diffs in chat messages#11316roomote-v0[bot] wants to merge 1 commit intomainfrom
Conversation
Reviewed. The settings plumbing, types, UI toggle, and i18n look correct. Found 2 issues in the auto-expand
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| useEffect(() => { | ||
| if (!autoExpandDiffs) { | ||
| return | ||
| } | ||
|
|
||
| const newExpansions: Record<number, boolean> = {} | ||
|
|
||
| for (const msg of modifiedMessages) { | ||
| // Skip messages already tracked in expandedRows | ||
| if (expandedRows[msg.ts] !== undefined) { | ||
| continue | ||
| } | ||
|
|
||
| // Check if this message contains a diff tool | ||
| if (msg.text) { | ||
| try { | ||
| const tool = JSON.parse(msg.text) | ||
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | ||
| newExpansions[msg.ts] = true | ||
| } | ||
| } catch { | ||
| // Not valid JSON, skip | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(newExpansions).length > 0) { | ||
| setExpandedRows((prev) => ({ ...prev, ...newExpansions })) | ||
| } | ||
| }, [modifiedMessages, autoExpandDiffs, expandedRows, DIFF_TOOL_TYPES]) |
There was a problem hiding this comment.
Including expandedRows in the dependency array here means this effect re-runs on every expansion/collapse (user or auto), iterating all modifiedMessages and JSON.parse-ing each one. It also causes a double execution when it auto-expands: the setExpandedRows call updates expandedRows, which re-triggers the effect (the second run is a no-op but still wasteful). You can fix both problems by moving the logic into the functional updater of setExpandedRows and dropping expandedRows from deps:
| useEffect(() => { | |
| if (!autoExpandDiffs) { | |
| return | |
| } | |
| const newExpansions: Record<number, boolean> = {} | |
| for (const msg of modifiedMessages) { | |
| // Skip messages already tracked in expandedRows | |
| if (expandedRows[msg.ts] !== undefined) { | |
| continue | |
| } | |
| // Check if this message contains a diff tool | |
| if (msg.text) { | |
| try { | |
| const tool = JSON.parse(msg.text) | |
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | |
| newExpansions[msg.ts] = true | |
| } | |
| } catch { | |
| // Not valid JSON, skip | |
| } | |
| } | |
| } | |
| if (Object.keys(newExpansions).length > 0) { | |
| setExpandedRows((prev) => ({ ...prev, ...newExpansions })) | |
| } | |
| }, [modifiedMessages, autoExpandDiffs, expandedRows, DIFF_TOOL_TYPES]) | |
| useEffect(() => { | |
| if (!autoExpandDiffs) { | |
| return | |
| } | |
| setExpandedRows((prev) => { | |
| const newExpansions: Record<number, boolean> = {} | |
| for (const msg of modifiedMessages) { | |
| if (prev[msg.ts] !== undefined) { | |
| continue | |
| } | |
| if (msg.text) { | |
| try { | |
| const tool = JSON.parse(msg.text) | |
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | |
| newExpansions[msg.ts] = true | |
| } | |
| } catch { | |
| // Not valid JSON, skip | |
| } | |
| } | |
| } | |
| if (Object.keys(newExpansions).length > 0) { | |
| return { ...prev, ...newExpansions } | |
| } | |
| return prev | |
| }) | |
| }, [modifiedMessages, autoExpandDiffs, DIFF_TOOL_TYPES]) |
Fix it with Roo Code or mention @roomote and request a fix.
| useEffect(() => { | ||
| if (!autoExpandDiffs) { | ||
| return | ||
| } | ||
|
|
||
| const newExpansions: Record<number, boolean> = {} | ||
|
|
||
| for (const msg of modifiedMessages) { | ||
| // Skip messages already tracked in expandedRows | ||
| if (expandedRows[msg.ts] !== undefined) { | ||
| continue | ||
| } | ||
|
|
||
| // Check if this message contains a diff tool | ||
| if (msg.text) { | ||
| try { | ||
| const tool = JSON.parse(msg.text) | ||
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | ||
| newExpansions[msg.ts] = true | ||
| } | ||
| } catch { | ||
| // Not valid JSON, skip | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(newExpansions).length > 0) { | ||
| setExpandedRows((prev) => ({ ...prev, ...newExpansions })) | ||
| } | ||
| }, [modifiedMessages, autoExpandDiffs, expandedRows, DIFF_TOOL_TYPES]) |
There was a problem hiding this comment.
When this effect auto-expands diffs via setExpandedRows, the existing effect at lines 509-529 will detect those transitions (from undefined to true) and set stickyFollowRef.current = false, disabling auto-scroll. It cannot distinguish between user-initiated and programmatic expansions. During an active task with autoExpandDiffs enabled, each new diff message gets auto-expanded, triggers that detection, and disables auto-scroll -- the user has to manually scroll to see subsequent messages, which undermines the purpose of this feature. Consider guarding against this by setting a ref flag (e.g. isAutoExpandingRef.current = true) before calling setExpandedRows here, and checking it in the expansion-tracking effect to skip the sticky-follow disable.
Fix it with Roo Code or mention @roomote and request a fix.
Adds a new boolean setting "autoExpandDiffs" (default: false) under Settings > UI. When enabled, file edit diffs in chat messages are automatically expanded instead of requiring a click on the collapsed filename bar. The CodeAccordion component already enforces a 300px max height with scrollbar, so auto-expanded diffs will not overwhelm the chat view. Changes: - packages/types: add autoExpandDiffs to GlobalSettings schema and ExtensionState - webview-ui context: add default and hydration for autoExpandDiffs - ChatView: add useEffect that auto-expands diff tool messages when setting is on - UISettings: add checkbox toggle for the new setting - SettingsView: wire the new setting through cached state - i18n: add English translation strings - Tests: update test fixtures for change-detection, unsaved-changes, UISettings Closes #10955
d1bddd3 to
0ac5f16
Compare
Related GitHub Issue
Closes: #10955
Description
This PR attempts to address Issue #10955. Feedback and guidance are welcome.
Adds a new boolean setting
autoExpandDiffs(default:false) under Settings > UI. When enabled, file edit diffs in chat messages (e.g. "Roo wants to edit this file") are automatically expanded instead of requiring a click on the collapsed filename bar.Implementation details:
packages/types): AddedautoExpandDiffstoglobalSettingsSchemaand theExtensionStatePick type so it flows through the state system.ExtensionStateContext): Added default value (false) and hydration handling.useEffectthat watchesgroupedMessagesand auto-expands any diff tool messages (editedExistingFile,appliedDiff,newFileCreated,insertContent,searchAndReplace,batchDiffApproval) when the setting is enabled. Only new (untracked) messages are expanded to avoid fighting with manual user toggles.cachedStatefor the save/discard workflow.The
CodeAccordioncomponent already enforces amax-h-[300px](300px) limit withoverflow-y: auto, so auto-expanded diffs will not take over the chat view -- anything taller than 300px gets a scrollbar.Test Procedure
Manual testing steps:
Pre-Submission Checklist
Documentation Updates
No documentation updates needed. The setting is exposed in Settings > UI with a clear label and description.