Browse Source

fix: prevent chatbox focus loss during automated file editing (#4574) (#5349)

* fix: prevent chatbox focus loss during automated file editing (#4574)

- Add preserveFocus: true to DiffViewProvider openDiffEditor method
- Implement comprehensive focus preservation during cursor positioning and scrolling
- Make scrollToFirstDiff async with focus restoration capabilities
- Update all tool files to use async scrollToFirstDiff
- Add comprehensive unit and E2E tests for focus preservation

Fixes #4574

* refactor: extract focus restoration logic and improve consistency

- Created restoreEditorFocus() helper method to eliminate code duplication
- Changed preserveFocus from false to true for consistent behavior
- Added error handling with silent logging for focus restoration failures
- Added null checks for undefined activeTextEditor cases

* Delete apps/vscode-e2e/src/suite/tools/focus-preservation.test.ts

It doesn't seem to be testing anything

* refactor: remove focus restoration logic to simplify diff editor interactions

* refactor: remove unnecessary await from scrollToFirstDiff calls

---------

Co-authored-by: Daniel <[email protected]>
Co-authored-by: Daniel Riccio <[email protected]>
Hannes Rudolph 8 months ago
parent
commit
ede228e152

+ 1 - 1
src/core/tools/applyDiffTool.ts

@@ -145,7 +145,7 @@ export async function applyDiffToolLegacy(
 			cline.diffViewProvider.editType = "modify"
 			await cline.diffViewProvider.open(relPath)
 			await cline.diffViewProvider.update(diffResult.content, true)
-			await cline.diffViewProvider.scrollToFirstDiff()
+			cline.diffViewProvider.scrollToFirstDiff()
 
 			// Check if file is write-protected
 			const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false

+ 1 - 1
src/core/tools/multiApplyDiffTool.ts

@@ -505,7 +505,7 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
 				cline.diffViewProvider.editType = "modify"
 				await cline.diffViewProvider.open(relPath)
 				await cline.diffViewProvider.update(originalContent!, true)
-				await cline.diffViewProvider.scrollToFirstDiff()
+				cline.diffViewProvider.scrollToFirstDiff()
 
 				// For batch operations, we've already gotten approval
 				const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false

+ 4 - 4
src/integrations/editor/DiffViewProvider.ts

@@ -122,7 +122,7 @@ export class DiffViewProvider {
 		}
 
 		// Place cursor at the beginning of the diff editor to keep it out of
-		// the way of the stream animation.
+		// the way of the stream animation, but do this without stealing focus
 		const beginningOfDocument = new vscode.Position(0, 0)
 		diffEditor.selection = new vscode.Selection(beginningOfDocument, beginningOfDocument)
 
@@ -137,7 +137,7 @@ export class DiffViewProvider {
 		// Update decorations.
 		this.activeLineController.setActiveLine(endLine)
 		this.fadedOverlayController.updateOverlayAfterLine(endLine, document.lineCount)
-		// Scroll to the current line.
+		// Scroll to the current line without stealing focus.
 		const ranges = this.activeDiffEditor?.visibleRanges
 		if (ranges && ranges.length > 0 && ranges[0].start.line < endLine && ranges[0].end.line > endLine) {
 			this.scrollEditorToLine(endLine)
@@ -504,7 +504,7 @@ export class DiffViewProvider {
 			// Pre-open the file as a text document to ensure it doesn't open in preview mode
 			// This fixes issues with files that have custom editor associations (like markdown preview)
 			vscode.window
-				.showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active })
+				.showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true })
 				.then(() => {
 					// Execute the diff command after ensuring the file is open as text
 					return vscode.commands.executeCommand(
@@ -552,7 +552,7 @@ export class DiffViewProvider {
 
 		for (const part of diffs) {
 			if (part.added || part.removed) {
-				// Found the first diff, scroll to it.
+				// Found the first diff, scroll to it without stealing focus.
 				this.activeDiffEditor.revealRange(
 					new vscode.Range(lineCount, 0, lineCount, 0),
 					vscode.TextEditorRevealType.InCenter,

+ 3 - 3
src/integrations/editor/__tests__/DiffViewProvider.spec.ts

@@ -176,7 +176,7 @@ describe("DiffViewProvider", () => {
 			// Mock showTextDocument to track when it's called
 			vi.mocked(vscode.window.showTextDocument).mockImplementation(async (uri, options) => {
 				callOrder.push("showTextDocument")
-				expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active })
+				expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true })
 				return mockEditor as any
 			})
 
@@ -208,10 +208,10 @@ describe("DiffViewProvider", () => {
 			// Verify that showTextDocument was called before executeCommand
 			expect(callOrder).toEqual(["showTextDocument", "executeCommand"])
 
-			// Verify that showTextDocument was called with preview: false
+			// Verify that showTextDocument was called with preview: false and preserveFocus: true
 			expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
 				expect.objectContaining({ fsPath: `${mockCwd}/test.md` }),
-				{ preview: false, viewColumn: vscode.ViewColumn.Active },
+				{ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true },
 			)
 
 			// Verify that the diff command was executed