Skip to content

Conversation

@kycutler
Copy link
Contributor

@kycutler kycutler commented Jan 9, 2026

Copilot AI review requested due to automatic review settings January 9, 2026 01:07
@vs-code-engineering vs-code-engineering bot added this to the January 2026 milestone Jan 9, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds "Add Element to Chat" functionality to the integrated browser, enabling users to select elements from browser views and attach them to chat conversations. The PR refactors the browser elements service architecture from using a BrowserType enum to a more flexible IBrowserTargetLocator interface that supports both webviews and browser views.

Key changes:

  • Replaced BrowserType enum with IBrowserTargetLocator interface for more flexible browser target identification
  • Added element selection functionality to integrated browser editor with new SelectElementAction and selectElement() method
  • Extracted duplicate getDisplayNameFromOuterHTML function to a shared utility in the common layer

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/vs/platform/browserElements/common/browserElements.ts Replaced BrowserType enum with IBrowserTargetLocator interface; added shared getDisplayNameFromOuterHTML utility function
src/vs/platform/browserElements/electron-main/nativeBrowserElementsMainService.ts Updated to use IBrowserTargetLocator; refactored target finding logic to support both webview and browser view locators; removed magic number for title bar height
src/vs/workbench/services/browserElements/browser/browserElementsService.ts Updated service interface signatures to use IBrowserTargetLocator
src/vs/workbench/services/browserElements/browser/webBrowserElementsService.ts Updated method signatures and added async keyword for consistency
src/vs/workbench/services/browserElements/electron-browser/browserElementsService.ts Updated to pass IBrowserTargetLocator to underlying service
src/vs/workbench/contrib/chat/browser/attachments/simpleBrowserEditorOverlay.ts Refactored to use webview IDs instead of URIs; removed duplicate getDisplayNameFromOuterHTML function; added title bar height offset
src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts Added selectElement functionality with proper cleanup, context key management, and cancellation support
src/vs/workbench/contrib/browserView/electron-browser/browserViewActions.ts Added SelectElementAction; removed precondition from ReloadAction; adjusted action ordering
src/vs/platform/browserView/electron-main/browserView.ts Added webContents getter for external access
src/vs/platform/browserView/electron-main/browserViewMainService.ts Added tryGetBrowserView method to retrieve browser views by ID
Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/chat/browser/attachments/simpleBrowserEditorOverlay.ts:256

  • The magic number 32.4 representing the title bar height should be extracted to a named constant to improve code maintainability and clarity. This value appears to be browser-specific UI measurement that could change.
			height: editorContainerPosition.height - 32.4,

lszomoru
lszomoru previously approved these changes Jan 9, 2026
@lszomoru
Copy link
Member

lszomoru commented Jan 9, 2026

@kycutler, PR build is 🔴

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants