From a701cdcef1a1f669c9436728d56faa5e718b2b76 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Thu, 12 Feb 2026 20:51:27 +0100 Subject: [PATCH] refactor views and pane composite parts (#295015) * refactor views and pane composite parts * simplify --- .../browser/parts/paneCompositePart.ts | 9 ++--- .../browser/parts/paneCompositePartService.ts | 9 +++++ src/vs/workbench/common/views.ts | 10 +++--- .../agentSessions/agentSessionsActions.ts | 5 +-- .../contrib/terminal/browser/terminalGroup.ts | 5 +-- .../panecomposite/browser/panecomposite.ts | 5 +++ .../views/browser/viewDescriptorService.ts | 2 +- .../services/views/browser/viewsService.ts | 36 ++++--------------- .../parts/activitybar/activitybarPart.test.ts | 3 +- .../test/browser/workbenchTestServices.ts | 12 +++++-- 10 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/vs/workbench/browser/parts/paneCompositePart.ts b/src/vs/workbench/browser/parts/paneCompositePart.ts index a431fe643f7..e46ba514858 100644 --- a/src/vs/workbench/browser/parts/paneCompositePart.ts +++ b/src/vs/workbench/browser/parts/paneCompositePart.ts @@ -12,7 +12,7 @@ import { IPaneComposite } from '../../common/panecomposite.js'; import { IViewDescriptorService, ViewContainerLocation } from '../../common/views.js'; import { DisposableStore, MutableDisposable } from '../../../base/common/lifecycle.js'; import { IView } from '../../../base/browser/ui/grid/grid.js'; -import { IWorkbenchLayoutService, Parts } from '../../services/layout/browser/layoutService.js'; +import { IWorkbenchLayoutService, Parts, SINGLE_WINDOW_PARTS } from '../../services/layout/browser/layoutService.js'; import { CompositePart, ICompositePartOptions, ICompositeTitleLabel } from './compositePart.js'; import { IPaneCompositeBarOptions, PaneCompositeBar } from './paneCompositeBar.js'; import { Dimension, EventHelper, trackFocus, $, addDisposableListener, EventType, prepend, getWindow } from '../../../base/browser/dom.js'; @@ -48,7 +48,8 @@ export enum CompositeBarPosition { export interface IPaneCompositePart extends IView { - readonly partId: Parts.PANEL_PART | Parts.AUXILIARYBAR_PART | Parts.SIDEBAR_PART; + readonly partId: SINGLE_WINDOW_PARTS; + readonly registryId: string; readonly onDidPaneCompositeOpen: Event; readonly onDidPaneCompositeClose: Event; @@ -132,7 +133,7 @@ export abstract class AbstractPaneCompositePart extends CompositePart, @@ -142,7 +143,7 @@ export abstract class AbstractPaneCompositePart extends CompositePart Event.map(this.paneCompositeParts.get(loc)!.onDidPaneCompositeClose, composite => { return { composite, viewContainerLocation: loc }; }, eventDisposables))); } + getRegistryId(viewContainerLocation: ViewContainerLocation): string { + return this.getPartByLocation(viewContainerLocation).registryId; + } + + getPartId(viewContainerLocation: ViewContainerLocation): SINGLE_WINDOW_PARTS { + return this.getPartByLocation(viewContainerLocation).partId; + } + openPaneComposite(id: string | undefined, viewContainerLocation: ViewContainerLocation, focus?: boolean): Promise { return this.getPartByLocation(viewContainerLocation).openPaneComposite(id, focus); } diff --git a/src/vs/workbench/common/views.ts b/src/vs/workbench/common/views.ts index 0a903822543..71d44300073 100644 --- a/src/vs/workbench/common/views.ts +++ b/src/vs/workbench/common/views.ts @@ -42,8 +42,6 @@ export const enum ViewContainerLocation { AuxiliaryBar } -export const ViewContainerLocations = [ViewContainerLocation.Sidebar, ViewContainerLocation.Panel, ViewContainerLocation.AuxiliaryBar]; - export function ViewContainerLocationToString(viewContainerLocation: ViewContainerLocation) { switch (viewContainerLocation) { case ViewContainerLocation.Sidebar: return 'sidebar'; @@ -175,9 +173,9 @@ export interface IViewContainersRegistry { getViewContainerLocation(container: ViewContainer): ViewContainerLocation; /** - * Return the default view container from the given location + * Return the default view containers from the given location */ - getDefaultViewContainer(location: ViewContainerLocation): ViewContainer | undefined; + getDefaultViewContainers(location: ViewContainerLocation): ViewContainer[]; } interface ViewOrderDelegate { @@ -250,8 +248,8 @@ class ViewContainersRegistryImpl extends Disposable implements IViewContainersRe return [...this.viewContainers.keys()].filter(location => this.getViewContainers(location).filter(viewContainer => viewContainer?.id === container.id).length > 0)[0]; } - getDefaultViewContainer(location: ViewContainerLocation): ViewContainer | undefined { - return this.defaultViewContainers.find(viewContainer => this.getViewContainerLocation(viewContainer) === location); + getDefaultViewContainers(location: ViewContainerLocation): ViewContainer[] { + return this.defaultViewContainers.filter(viewContainer => this.getViewContainerLocation(viewContainer) === location); } } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts index c07c6e5111a..56f51542f99 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts @@ -15,7 +15,6 @@ import { IChatEditorOptions } from '../widgetHosts/editor/chatEditor.js'; import { ChatViewId, IChatWidgetService } from '../chat.js'; import { ACTIVE_GROUP, AUX_WINDOW_GROUP, PreferredGroup, SIDE_GROUP } from '../../../../services/editor/common/editorService.js'; import { IViewDescriptorService, ViewContainerLocation } from '../../../../common/views.js'; -import { getPartByLocation } from '../../../../services/views/browser/viewsService.js'; import { IWorkbenchLayoutService, Position } from '../../../../services/layout/browser/layoutService.js'; import { IAgentSessionsService } from './agentSessionsService.js'; import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; @@ -35,6 +34,7 @@ import { KeybindingWeight } from '../../../../../platform/keybinding/common/keyb import { KeyCode, KeyMod } from '../../../../../base/common/keyCodes.js'; import { coalesce } from '../../../../../base/common/arrays.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { IPaneCompositePartService } from '../../../../services/panecomposite/browser/panecomposite.js'; const AGENT_SESSIONS_CATEGORY = localize2('chatSessions', "Chat Agent Sessions"); @@ -834,6 +834,7 @@ abstract class UpdateChatViewWidthAction extends Action2 { const viewDescriptorService = accessor.get(IViewDescriptorService); const configurationService = accessor.get(IConfigurationService); const viewsService = accessor.get(IViewsService); + const paneCompositeService = accessor.get(IPaneCompositePartService); const chatLocation = viewDescriptorService.getViewLocationById(ChatViewId); if (typeof chatLocation !== 'number') { @@ -880,7 +881,7 @@ abstract class UpdateChatViewWidthAction extends Action2 { return; // location does not allow for resize (panel top or bottom) } - const part = getPartByLocation(chatLocation); + const part = paneCompositeService.getPartId(chatLocation); let currentSize = layoutService.getSize(part); const chatViewDefaultWidth = 300; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalGroup.ts b/src/vs/workbench/contrib/terminal/browser/terminalGroup.ts index bead415e2a7..da3402de126 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalGroup.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalGroup.ts @@ -14,9 +14,9 @@ import { ViewContainerLocation, IViewDescriptorService } from '../../../common/v import { IShellLaunchConfig, ITerminalTabLayoutInfoById, TerminalLocation } from '../../../../platform/terminal/common/terminal.js'; import { TerminalStatus } from './terminalStatusList.js'; import { getWindow } from '../../../../base/browser/dom.js'; -import { getPartByLocation } from '../../../services/views/browser/viewsService.js'; import { asArray } from '../../../../base/common/arrays.js'; import { hasKey, isNumber, type SingleOrMany } from '../../../../base/common/types.js'; +import { IPaneCompositePartService } from '../../../services/panecomposite/browser/panecomposite.js'; const enum Constants { /** @@ -278,6 +278,7 @@ export class TerminalGroup extends Disposable implements ITerminalGroup { shellLaunchConfigOrInstance: IShellLaunchConfig | ITerminalInstance | undefined, @ITerminalConfigurationService private readonly _terminalConfigurationService: ITerminalConfigurationService, @ITerminalInstanceService private readonly _terminalInstanceService: ITerminalInstanceService, + @IPaneCompositePartService private readonly _paneCompositePartService: IPaneCompositePartService, @IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService, @IViewDescriptorService private readonly _viewDescriptorService: IViewDescriptorService, @IInstantiationService private readonly _instantiationService: IInstantiationService @@ -612,7 +613,7 @@ export class TerminalGroup extends Disposable implements ITerminalGroup { resizeAmount *= -1; } - this._layoutService.resizePart(getPartByLocation(this._terminalLocation), resizeAmount, resizeAmount); + this._layoutService.resizePart(this._paneCompositePartService.getPartId(this._terminalLocation), resizeAmount, resizeAmount); } else { this._splitPaneContainer.resizePane(this._activeInstanceIndex, direction, resizeAmount); } diff --git a/src/vs/workbench/services/panecomposite/browser/panecomposite.ts b/src/vs/workbench/services/panecomposite/browser/panecomposite.ts index de8d0648f97..3e4e569e3c2 100644 --- a/src/vs/workbench/services/panecomposite/browser/panecomposite.ts +++ b/src/vs/workbench/services/panecomposite/browser/panecomposite.ts @@ -9,6 +9,7 @@ import { PaneCompositeDescriptor } from '../../../browser/panecomposite.js'; import { IProgressIndicator } from '../../../../platform/progress/common/progress.js'; import { IPaneComposite } from '../../../common/panecomposite.js'; import { ViewContainerLocation } from '../../../common/views.js'; +import { SINGLE_WINDOW_PARTS } from '../../layout/browser/layoutService.js'; export const IPaneCompositePartService = createDecorator('paneCompositePartService'); @@ -19,6 +20,10 @@ export interface IPaneCompositePartService { readonly onDidPaneCompositeOpen: Event<{ composite: IPaneComposite; viewContainerLocation: ViewContainerLocation }>; readonly onDidPaneCompositeClose: Event<{ composite: IPaneComposite; viewContainerLocation: ViewContainerLocation }>; + getRegistryId(viewContainerLocation: ViewContainerLocation): string; + + getPartId(viewContainerLocation: ViewContainerLocation): SINGLE_WINDOW_PARTS; + /** * Opens a viewlet with the given identifier and pass keyboard focus to it if specified. */ diff --git a/src/vs/workbench/services/views/browser/viewDescriptorService.ts b/src/vs/workbench/services/views/browser/viewDescriptorService.ts index a25cf00bfc9..62cf852a83b 100644 --- a/src/vs/workbench/services/views/browser/viewDescriptorService.ts +++ b/src/vs/workbench/services/views/browser/viewDescriptorService.ts @@ -308,7 +308,7 @@ export class ViewDescriptorService extends Disposable implements IViewDescriptor } getDefaultViewContainer(location: ViewContainerLocation): ViewContainer | undefined { - return this.viewContainersRegistry.getDefaultViewContainer(location); + return this.viewContainersRegistry.getDefaultViewContainers(location)[0]; } canMoveViews(): boolean { diff --git a/src/vs/workbench/services/views/browser/viewsService.ts b/src/vs/workbench/services/views/browser/viewsService.ts index 4938ebb966b..69c6c0c252a 100644 --- a/src/vs/workbench/services/views/browser/viewsService.ts +++ b/src/vs/workbench/services/views/browser/viewsService.ts @@ -23,7 +23,7 @@ import { IThemeService } from '../../../../platform/theme/common/themeService.js import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js'; import { IExtensionService } from '../../extensions/common/extensions.js'; import { IWorkspaceContextService } from '../../../../platform/workspace/common/workspace.js'; -import { PaneCompositeDescriptor, PaneCompositeRegistry, Extensions as PaneCompositeExtensions, PaneComposite } from '../../../browser/panecomposite.js'; +import { PaneCompositeDescriptor, PaneCompositeRegistry, PaneComposite } from '../../../browser/panecomposite.js'; import { IWorkbenchLayoutService, Parts } from '../../layout/browser/layoutService.js'; import { URI } from '../../../../base/common/uri.js'; import { IProgressIndicator } from '../../../../platform/progress/common/progress.js'; @@ -150,7 +150,7 @@ export class ViewsService extends Disposable implements IViewsService { // Open view container if part is visible and there is only one view container in location if ( - this.layoutService.isVisible(getPartByLocation(to)) && + this.layoutService.isVisible(this.paneCompositeService.getPartId(to)) && this.viewDescriptorService.getViewContainersByLocation(to).filter(vc => this.isViewContainerActive(vc.id)).length === 1 ) { this.openViewContainer(viewContainer.id); @@ -257,7 +257,7 @@ export class ViewsService extends Disposable implements IViewsService { const viewContainerLocation = this.viewDescriptorService.getViewContainerLocation(viewContainer); const isActive = viewContainerLocation !== null && this.paneCompositeService.getActivePaneComposite(viewContainerLocation); if (viewContainerLocation !== null) { - return isActive ? this.layoutService.setPartHidden(true, getPartByLocation(viewContainerLocation)) : undefined; + return isActive ? this.layoutService.setPartHidden(true, this.paneCompositeService.getPartId(viewContainerLocation)) : undefined; } } } @@ -528,7 +528,7 @@ export class ViewsService extends Disposable implements IViewsService { editorGroupService.activeGroup.focus(); } else if (viewLocation !== null) { // otherwise hide the part where the view lives if focused - layoutService.setPartHidden(true, getPartByLocation(viewLocation)); + layoutService.setPartHidden(true, that.paneCompositeService.getPartId(viewLocation)); } } else { await viewsService.openView(viewDescriptor.id, !options?.preserveFocus); @@ -670,7 +670,7 @@ export class ViewsService extends Disposable implements IViewsService { } } - Registry.as(getPaneCompositeExtension(viewContainerLocation)).registerPaneComposite(PaneCompositeDescriptor.create( + Registry.as(this.paneCompositeService.getRegistryId(viewContainerLocation)).registerPaneComposite(PaneCompositeDescriptor.create( PaneContainer, viewContainer.id, typeof viewContainer.title === 'string' ? viewContainer.title : viewContainer.title.value, @@ -682,7 +682,7 @@ export class ViewsService extends Disposable implements IViewsService { } private deregisterPaneComposite(viewContainer: ViewContainer, viewContainerLocation: ViewContainerLocation): void { - Registry.as(getPaneCompositeExtension(viewContainerLocation)).deregisterPaneComposite(viewContainer.id); + Registry.as(this.paneCompositeService.getRegistryId(viewContainerLocation)).deregisterPaneComposite(viewContainer.id); } private createViewPaneContainer(element: HTMLElement, viewContainer: ViewContainer, viewContainerLocation: ViewContainerLocation, disposables: DisposableStore, instantiationService: IInstantiationService): ViewPaneContainer { @@ -712,28 +712,4 @@ export class ViewsService extends Disposable implements IViewsService { function getEnabledViewContainerContextKey(viewContainerId: string): string { return `viewContainer.${viewContainerId}.enabled`; } -function getPaneCompositeExtension(viewContainerLocation: ViewContainerLocation): string { - switch (viewContainerLocation) { - case ViewContainerLocation.AuxiliaryBar: - return PaneCompositeExtensions.Auxiliary; - case ViewContainerLocation.Panel: - return PaneCompositeExtensions.Panels; - case ViewContainerLocation.Sidebar: - default: - return PaneCompositeExtensions.Viewlets; - } -} - -export function getPartByLocation(viewContainerLocation: ViewContainerLocation): Parts.AUXILIARYBAR_PART | Parts.SIDEBAR_PART | Parts.PANEL_PART { - switch (viewContainerLocation) { - case ViewContainerLocation.AuxiliaryBar: - return Parts.AUXILIARYBAR_PART; - case ViewContainerLocation.Panel: - return Parts.PANEL_PART; - case ViewContainerLocation.Sidebar: - default: - return Parts.SIDEBAR_PART; - } -} - registerSingleton(IViewsService, ViewsService, InstantiationType.Eager /* Eager because it registers viewlets and panels in the constructor which are required during workbench layout */); diff --git a/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts b/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts index e28ed97e587..2cfc1d0f8c2 100644 --- a/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts +++ b/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts @@ -18,13 +18,14 @@ import { IConfigurationChangeEvent } from '../../../../../platform/configuration import { IPaneCompositePart } from '../../../../browser/parts/paneCompositePart.js'; import { Event, Emitter } from '../../../../../base/common/event.js'; import { IPaneComposite } from '../../../../common/panecomposite.js'; -import { PaneCompositeDescriptor } from '../../../../browser/panecomposite.js'; +import { Extensions, PaneCompositeDescriptor } from '../../../../browser/panecomposite.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ViewContainerLocation } from '../../../../common/views.js'; class StubPaneCompositePart implements IPaneCompositePart { declare readonly _serviceBrand: undefined; readonly partId = Parts.SIDEBAR_PART; + readonly registryId = Extensions.Viewlets; element: HTMLElement = undefined!; minimumWidth = 0; maximumWidth = 0; diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index 4d70e2e4a4b..5f9844e71e8 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -114,7 +114,7 @@ import { IWorkspaceTrustManagementService, IWorkspaceTrustRequestService } from import { TestWorkspace } from '../../../platform/workspace/test/common/testWorkspace.js'; import { IEnterWorkspaceResult, IRecent, IRecentlyOpened, IWorkspaceFolderCreationData, IWorkspacesService } from '../../../platform/workspaces/common/workspaces.js'; import { EditorPaneDescriptor, IEditorPaneRegistry } from '../../browser/editor.js'; -import { PaneComposite, PaneCompositeDescriptor } from '../../browser/panecomposite.js'; +import { PaneComposite, PaneCompositeDescriptor, Extensions as PaneCompositeExtensions } from '../../browser/panecomposite.js'; import { Part } from '../../browser/part.js'; import { DEFAULT_EDITOR_PART_OPTIONS, EditorServiceImpl, IEditorGroupsView, IEditorGroupTitleHeight, IEditorGroupView } from '../../browser/parts/editor/editor.js'; import { EditorPane } from '../../browser/parts/editor/editorPane.js'; @@ -164,7 +164,7 @@ import { IHistoryService } from '../../services/history/common/history.js'; import { IHostService, IToastOptions, IToastResult } from '../../services/host/browser/host.js'; import { LabelService } from '../../services/label/common/labelService.js'; import { ILanguageDetectionService } from '../../services/languageDetection/common/languageDetectionWorkerService.js'; -import { IPartVisibilityChangeEvent, IWorkbenchLayoutService, PanelAlignment, Position as PartPosition, Parts } from '../../services/layout/browser/layoutService.js'; +import { IPartVisibilityChangeEvent, IWorkbenchLayoutService, PanelAlignment, Position as PartPosition, Parts, SINGLE_WINDOW_PARTS } from '../../services/layout/browser/layoutService.js'; import { ILifecycleService, InternalBeforeShutdownEvent, IWillShutdownEventJoiner, ShutdownReason, WillShutdownEvent } from '../../services/lifecycle/common/lifecycle.js'; import { IPaneCompositePartService } from '../../services/panecomposite/browser/panecomposite.js'; import { IPathService } from '../../services/path/common/pathService.js'; @@ -724,6 +724,12 @@ export class TestPaneCompositeService extends Disposable implements IPaneComposi this.onDidPaneCompositeClose = Event.any(...([ViewContainerLocation.Panel, ViewContainerLocation.Sidebar].map(loc => Event.map(this.parts.get(loc)!.onDidPaneCompositeClose, composite => { return { composite, viewContainerLocation: loc }; })))); } + getPartId(viewContainerLocation: ViewContainerLocation): SINGLE_WINDOW_PARTS { + return this.getPartByLocation(viewContainerLocation).partId; + } + getRegistryId(viewContainerLocation: ViewContainerLocation): string { + return this.getPartByLocation(viewContainerLocation).registryId; + } openPaneComposite(id: string | undefined, viewContainerLocation: ViewContainerLocation, focus?: boolean): Promise { return this.getPartByLocation(viewContainerLocation).openPaneComposite(id, focus); } @@ -772,6 +778,7 @@ export class TestSideBarPart implements IPaneCompositePart { onDidViewletCloseEmitter = new Emitter(); readonly partId = Parts.SIDEBAR_PART; + readonly registryId = PaneCompositeExtensions.Viewlets; element: HTMLElement = undefined!; minimumWidth = 0; maximumWidth = 0; @@ -809,6 +816,7 @@ export class TestPanelPart implements IPaneCompositePart { onDidPaneCompositeOpen = new Emitter().event; onDidPaneCompositeClose = new Emitter().event; readonly partId = Parts.AUXILIARYBAR_PART; + readonly registryId = PaneCompositeExtensions.Auxiliary; async openPaneComposite(id?: string, focus?: boolean): Promise { return undefined; } getPaneComposite(id: string): any { return activeViewlet; }