Skip to content

Conversation

@lszomoru
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings January 21, 2026 09:36
@lszomoru lszomoru enabled auto-merge (squash) January 21, 2026 09:36
@lszomoru lszomoru self-assigned this Jan 21, 2026
@vs-code-engineering vs-code-engineering bot added this to the January 2026 milestone Jan 21, 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 refactors the floating editor toolbar by extracting the widget rendering logic from FloatingEditorToolbar into a new reusable FloatingEditorToolbarWidget class. This separation improves code organization and allows the widget to be reused in other contexts.

Changes:

  • Created new FloatingEditorToolbarWidget class that encapsulates the toolbar widget's element creation, menu observation, and rendering logic
  • Refactored FloatingEditorToolbar to instantiate and manage the widget, delegating the toolbar UI creation to the new widget class
  • Modified the element lifecycle so it's created once in the widget constructor rather than being recreated on each menu change
Comments suppressed due to low confidence (1)

src/vs/editor/contrib/floatingMenu/browser/floatingMenu.ts:102

  • Each time the toolbar is recreated, it's being added to the same this.element container without clearing previous content. This will cause the old toolbar's DOM nodes to remain in the element even though they're disposed, leading to DOM pollution. The element should be cleared before creating a new toolbar, or the toolbar should be created only once and updated reactively.
			const toolbar = instantiationService.createInstance(MenuWorkbenchToolBar, this.element, MenuId.EditorContent, {


// Toolbar
const toolbar = instantiationService.createInstance(MenuWorkbenchToolBar, container.root, MenuId.EditorContent, {
const toolbar = instantiationService.createInstance(MenuWorkbenchToolBar, this.element, MenuId.EditorContent, {
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The menuId parameter is hardcoded as MenuId.EditorContent instead of using the _menuId parameter that is passed to the constructor. This makes the _menuId parameter effectively unused for the toolbar configuration, reducing the reusability of this widget. Use _menuId instead of MenuId.EditorContent.

Suggested change
const toolbar = instantiationService.createInstance(MenuWorkbenchToolBar, this.element, MenuId.EditorContent, {
const toolbar = instantiationService.createInstance(MenuWorkbenchToolBar, this.element, _menuId, {

Copilot uses AI. Check for mistakes.
@lszomoru lszomoru merged commit 406f3f0 into main Jan 21, 2026
27 of 28 checks passed
@lszomoru lszomoru deleted the lszomoru/voluntary-possum branch January 21, 2026 09:59
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.

3 participants