diff --git a/build/lib/stylelint/vscode-known-variables.json b/build/lib/stylelint/vscode-known-variables.json index 593a6366dbd202..96eefd71ed6f90 100644 --- a/build/lib/stylelint/vscode-known-variables.json +++ b/build/lib/stylelint/vscode-known-variables.json @@ -977,6 +977,7 @@ "others": [ "--action-widget-close-start-opacity", "--action-widget-close-start-transform", + "--activity-bar-action-gap", "--activity-bar-action-height", "--activity-bar-icon-size", "--activity-bar-width", diff --git a/src/vs/workbench/browser/media/floatingPanels.css b/src/vs/workbench/browser/media/floatingPanels.css index 263a02c4af5a56..f82d1a8464ebb3 100644 --- a/src/vs/workbench/browser/media/floatingPanels.css +++ b/src/vs/workbench/browser/media/floatingPanels.css @@ -194,10 +194,11 @@ margin-top: var(--vscode-spacing-size20); } -/* At the default (non-compact) size, separate the activity bar items with an 8px gap - * so they read as distinct floating targets. Compact keeps the tighter default stack. */ -.monaco-workbench.floating-panels .part.activitybar:not(.compact) > .content .monaco-action-bar .action-item + .action-item { - margin-top: var(--vscode-spacing-size80); +/* Separate the activity bar items so they read as distinct floating targets. The gap is + * published by activitybarPart.ts as `--activity-bar-action-gap` (0px at the compact size) + * so that it stays in step with the overflow computation. */ +.monaco-workbench.floating-panels .part.activitybar > .content .monaco-action-bar .action-item + .action-item { + margin-top: var(--activity-bar-action-gap, 0px); } /* Inset and vertically center status bar items within the full-width bottom rail. */ diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts index f5d6a69b845fb0..3e2077339f26f4 100644 --- a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts +++ b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts @@ -55,6 +55,13 @@ export class ActivitybarPart extends Part { static readonly FLOATING_ACTIVITYBAR_WIDTH = 36; static readonly FLOATING_COMPACT_ACTIVITYBAR_WIDTH = 28; + /** + * Vertical gap between activity bar items at the default size under the floating + * panels experiment. Published to CSS as `--activity-bar-action-gap` so that the + * stylesheet and the overflow computation cannot drift apart. + */ + static readonly FLOATING_ACTION_GAP = 8; + static readonly ICON_SIZE = 24; static readonly COMPACT_ICON_SIZE = 16; @@ -85,7 +92,7 @@ export class ActivitybarPart extends Part { return this._isCompact ? ActivitybarPart.COMPACT_ACTIVITYBAR_WIDTH : ActivitybarPart.ACTIVITYBAR_WIDTH; } - /** The action (item) height that drives visible item sizing and the composite bar overflow size. */ + /** The action (item) height that drives visible item sizing. */ private get actionHeight(): number { if (this._isCompact) { return ActivitybarPart.COMPACT_ACTION_HEIGHT; @@ -93,6 +100,24 @@ export class ActivitybarPart extends Part { return this.layoutService.isFloatingPanelsEnabled() ? ActivitybarPart.FLOATING_ACTION_HEIGHT : ActivitybarPart.ACTION_HEIGHT; } + /** + * Vertical gap rendered between two adjacent items. Only the floating panels + * experiment separates items, and only at the default size. + */ + private get actionGap(): number { + return this.layoutService.isFloatingPanelsEnabled() && !this._isCompact ? ActivitybarPart.FLOATING_ACTION_GAP : 0; + } + + /** + * The vertical space a single item occupies in the bar (its height plus the gap that + * separates it from the next one). This drives the overflow computation, so it has to + * track the current activity bar size, otherwise items collapse into the overflow menu + * prematurely. + */ + private get compositeSize(): number { + return this.actionHeight + this.actionGap; + } + private get floatingHorizontalGutter(): number { if (!this.layoutService.isFloatingPanelsEnabled()) { return 0; @@ -164,6 +189,7 @@ export class ActivitybarPart extends Part { this.layoutService.mainContainer.classList.toggle('activitybar-compact', this._isCompact); this.element.style.setProperty('--activity-bar-width', `${this.baseWidth}px`); this.element.style.setProperty('--activity-bar-action-height', `${this.actionHeight}px`); + this.element.style.setProperty('--activity-bar-action-gap', `${this.actionGap}px`); this.element.style.setProperty('--activity-bar-icon-size', `${this._isCompact ? ActivitybarPart.COMPACT_ICON_SIZE : ActivitybarPart.ICON_SIZE}px`); } } @@ -184,7 +210,7 @@ export class ActivitybarPart extends Part { } private createCompositeBar(): PaneCompositeBar { - const actionHeight = this.actionHeight; + const compositeSize = this.compositeSize; const iconSize = this._isCompact ? ActivitybarPart.COMPACT_ICON_SIZE : ActivitybarPart.ICON_SIZE; return this.instantiationService.createInstance(ActivityBarCompositeBar, this.location, { @@ -201,7 +227,7 @@ export class ActivitybarPart extends Part { preventLoopNavigation: true, recomputeSizes: false, fillExtraContextMenuActions: (actions, e?: MouseEvent | GestureEvent) => { }, - compositeSize: 52, + compositeSize, colors: (theme: IColorTheme) => ({ activeForegroundColor: theme.getColor(ACTIVITY_BAR_FOREGROUND), inactiveForegroundColor: theme.getColor(ACTIVITY_BAR_INACTIVE_FOREGROUND), @@ -212,7 +238,7 @@ export class ActivitybarPart extends Part { dragAndDropBorder: theme.getColor(ACTIVITY_BAR_DRAG_AND_DROP_BORDER), activeBackgroundColor: undefined, inactiveBackgroundColor: undefined, activeBorderBottomColor: undefined, }), - overflowActionSize: actionHeight, + overflowActionSize: compositeSize, }, Parts.ACTIVITYBAR_PART, this.paneCompositePart, true); } @@ -306,8 +332,8 @@ export class ActivitybarPart extends Part { // Layout contents const contentAreaSize = super.layoutContents(contentWidth, contentHeight).contentSize; - // Layout composite bar - this.compositeBar.value?.layout(contentWidth, contentAreaSize.height); + // The first item has no preceding gap, so give one gap back to the composite bar. + this.compositeBar.value?.layout(contentWidth, contentAreaSize.height + this.actionGap); } /** @@ -502,7 +528,7 @@ export class ActivityBarCompositeBar extends PaneCompositeBar { } if (this.globalCompositeBar) { if (this.options.orientation === ActionsOrientation.VERTICAL) { - height -= (this.globalCompositeBar.size() * this.options.overflowActionSize); + height -= this.globalCompositeBar.element.clientHeight; } else { width -= this.globalCompositeBar.element.clientWidth; } diff --git a/src/vs/workbench/browser/parts/globalCompositeBar.ts b/src/vs/workbench/browser/parts/globalCompositeBar.ts index 2187088a06dcd7..6aec4cf96b3b03 100644 --- a/src/vs/workbench/browser/parts/globalCompositeBar.ts +++ b/src/vs/workbench/browser/parts/globalCompositeBar.ts @@ -133,10 +133,6 @@ export class GlobalCompositeBar extends Disposable { this.globalActivityActionBar.focus(true); } - size(): number { - return this.globalActivityActionBar.viewItems.length; - } - getContextMenuActions(): IAction[] { return [toAction({ id: 'toggleAccountsVisibility', label: localize('accounts', "Accounts"), checked: this.accountsVisibilityPreference, run: () => this.accountsVisibilityPreference = !this.accountsVisibilityPreference })]; } 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 eda6fd12cf993f..46d6d84e0e5cac 100644 --- a/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts +++ b/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts @@ -10,18 +10,34 @@ import { TestConfigurationService } from '../../../../../platform/configuration/ import { TestColorTheme, TestThemeService } from '../../../../../platform/theme/test/common/testThemeService.js'; import { TestStorageService } from '../../../common/workbenchTestServices.js'; import { TestHostService, TestLayoutService } from '../../workbenchTestServices.js'; -import { ActivitybarPart } from '../../../../browser/parts/activitybar/activitybarPart.js'; +import { ActivitybarPart, ActivityBarCompositeBar } from '../../../../browser/parts/activitybar/activitybarPart.js'; import { IViewSize } from '../../../../../base/browser/ui/grid/grid.js'; import { LayoutSettings, Parts, Position } from '../../../../services/layout/browser/layoutService.js'; import { mainWindow } from '../../../../../base/browser/window.js'; import { IConfigurationChangeEvent } from '../../../../../platform/configuration/common/configuration.js'; import { IPaneCompositePart } from '../../../../browser/parts/paneCompositePart.js'; +import { IPaneCompositeBarOptions } from '../../../../browser/parts/paneCompositeBar.js'; import { Event, Emitter } from '../../../../../base/common/event.js'; import { IPaneComposite } from '../../../../common/panecomposite.js'; import { Extensions, PaneCompositeDescriptor } from '../../../../browser/panecomposite.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ViewContainerLocation } from '../../../../common/views.js'; import { ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND } from '../../../../common/theme.js'; +import { ActionsOrientation } from '../../../../../base/browser/ui/actionbar/actionbar.js'; +import { Dimension } from '../../../../../base/browser/dom.js'; + +interface ILayoutTestHarness { + menuBarContainer: HTMLElement | undefined; + globalCompositeBar: { element: HTMLElement } | undefined; + options: { orientation: ActionsOrientation }; + compositeBar: { layout: (dimension: Dimension) => void }; +} + +// `super.layout()` resolves through the prototype chain, so calling the extracted method +// against a harness still runs the real `PaneCompositeBar.layout` and hands the resulting +// dimension to `compositeBar`. +const activityBarCompositeBarLayout = Reflect.get(ActivityBarCompositeBar.prototype, 'layout') as (this: ILayoutTestHarness, width: number, height: number) => void; + class StubPaneCompositePart implements IPaneCompositePart { declare readonly _serviceBrand: undefined; @@ -74,7 +90,7 @@ suite('ActivitybarPart', () => { disposables.clear(); }); - function createActivitybarPart(compact: boolean, floatingPanelsEnabled = false, sideBarPosition = Position.LEFT, colors: { [id: string]: string | undefined } = {}): { part: ActivitybarPart; configService: TestConfigurationService; layoutService: TestFloatingPanelsLayoutService; hostService: TestHostService } { + function createActivitybarPart(compact: boolean, floatingPanelsEnabled = false, sideBarPosition = Position.LEFT, colors: { [id: string]: string | undefined } = {}, instantiationService?: IInstantiationService): { part: ActivitybarPart; configService: TestConfigurationService; layoutService: TestFloatingPanelsLayoutService; hostService: TestHostService } { const configService = new TestConfigurationService({ [LayoutSettings.ACTIVITY_BAR_COMPACT]: compact, [LayoutSettings.MODERN_UI]: floatingPanelsEnabled, @@ -92,7 +108,7 @@ suite('ActivitybarPart', () => { // Stub instantiation service—createCompositeBar is only called in show(), // which we skip in unit tests focused on dimensions / style behaviour. - const stubInstantiationService = { createInstance: () => { throw new Error('not expected'); } } as unknown as IInstantiationService; + const stubInstantiationService = instantiationService ?? { createInstance: () => { throw new Error('not expected'); } } as unknown as IInstantiationService; const part = disposables.add(new ActivitybarPart( ViewContainerLocation.Sidebar, @@ -295,6 +311,7 @@ suite('ActivitybarPart', () => { assert.strictEqual(el.style.getPropertyValue('--activity-bar-width'), `${ActivitybarPart.ACTIVITYBAR_WIDTH}px`); assert.strictEqual(el.style.getPropertyValue('--activity-bar-action-height'), `${ActivitybarPart.ACTION_HEIGHT}px`); + assert.strictEqual(el.style.getPropertyValue('--activity-bar-action-gap'), '0px'); assert.strictEqual(el.style.getPropertyValue('--activity-bar-icon-size'), `${ActivitybarPart.ICON_SIZE}px`); assert.strictEqual(el.classList.contains('compact'), false); }); @@ -308,6 +325,7 @@ suite('ActivitybarPart', () => { assert.strictEqual(el.style.getPropertyValue('--activity-bar-width'), `${ActivitybarPart.COMPACT_ACTIVITYBAR_WIDTH}px`); assert.strictEqual(el.style.getPropertyValue('--activity-bar-action-height'), `${ActivitybarPart.COMPACT_ACTION_HEIGHT}px`); + assert.strictEqual(el.style.getPropertyValue('--activity-bar-action-gap'), '0px'); assert.strictEqual(el.style.getPropertyValue('--activity-bar-icon-size'), `${ActivitybarPart.COMPACT_ICON_SIZE}px`); assert.strictEqual(el.classList.contains('compact'), true); }); @@ -321,6 +339,7 @@ suite('ActivitybarPart', () => { assert.strictEqual(el.style.getPropertyValue('--activity-bar-width'), `${ActivitybarPart.FLOATING_ACTIVITYBAR_WIDTH}px`); assert.strictEqual(el.style.getPropertyValue('--activity-bar-action-height'), `${ActivitybarPart.FLOATING_ACTION_HEIGHT}px`); + assert.strictEqual(el.style.getPropertyValue('--activity-bar-action-gap'), `${ActivitybarPart.FLOATING_ACTION_GAP}px`); assert.strictEqual(el.style.getPropertyValue('--activity-bar-icon-size'), `${ActivitybarPart.ICON_SIZE}px`); assert.strictEqual(el.classList.contains('compact'), false); }); @@ -438,5 +457,147 @@ suite('ActivitybarPart', () => { }); }); + // --- composite bar item sizing ------------------------------------------- + + // The composite bar decides how many activity icons fit before collapsing the rest into + // the overflow ("Additional Views") menu, so the size it is handed has to match the + // vertical space an item actually occupies in the current mode. + function capturedCompositeBarOptions(compact: boolean, floatingPanelsEnabled: boolean): IPaneCompositeBarOptions { + let captured: IPaneCompositeBarOptions | undefined; + const stubCompositeBar = { create: () => { }, layout: () => { }, dispose: () => { } }; + const { part } = createActivitybarPart(compact, floatingPanelsEnabled, Position.LEFT, {}, { + createInstance: (_descriptor: unknown, _location: unknown, options: IPaneCompositeBarOptions) => { + captured = options; + return stubCompositeBar; + } + } as unknown as IInstantiationService); + + const el = document.createElement('div'); + fixture.appendChild(el); + part.create(el); + part.show(); + + return captured!; + } + + test('composite bar item size tracks the rendered item stride in every mode', () => { + const sizesFor = (compact: boolean, floatingPanelsEnabled: boolean) => { + const { compositeSize, overflowActionSize } = capturedCompositeBarOptions(compact, floatingPanelsEnabled); + return { compositeSize, overflowActionSize }; + }; + + assert.deepStrictEqual( + { + classicDefault: sizesFor(false, false), + classicCompact: sizesFor(true, false), + modernDefault: sizesFor(false, true), + modernCompact: sizesFor(true, true), + }, + { + // Items stack flush against each other, so the stride is just the action height. + classicDefault: { compositeSize: 48, overflowActionSize: 48 }, + classicCompact: { compositeSize: 28, overflowActionSize: 28 }, + // Modern UI separates items with an 8px gap, but only at the default size. + modernDefault: { compositeSize: 44, overflowActionSize: 44 }, + modernCompact: { compositeSize: 28, overflowActionSize: 28 }, + } + ); + }); + + // The gap is rendered *between* items, so N items occupy `N * height + (N - 1) * gap`. + // `compositeSize` bakes a trailing gap into every item, which over-counts by exactly one + // gap, so `layout()` hands that gap back to the composite bar. Without it the last item + // is pushed into the overflow menu a gap earlier than it needs to be. + function compositeBarLayoutHeight(compact: boolean, floatingPanelsEnabled: boolean): number { + let layoutHeight = -1; + const stubCompositeBar = { + create: () => { }, + layout: (_width: number, height: number) => { layoutHeight = height; }, + dispose: () => { } + }; + const { part, layoutService } = createActivitybarPart(compact, floatingPanelsEnabled, Position.LEFT, {}, { + createInstance: () => stubCompositeBar + } as unknown as IInstantiationService); + + const el = document.createElement('div'); + fixture.appendChild(el); + part.create(el); + part.show(); + + // A visible title and status bar means neither edge is a window edge. + const visible = new Set([Parts.TITLEBAR_PART, Parts.STATUSBAR_PART]); + layoutService.isVisible = (partId: Parts) => visible.has(partId); + part.layout(100, 300); + + return layoutHeight; + } + + test('composite bar is given back the leading item gap it does not render', () => { + const margin = ActivitybarPart.FLOATING_MARGIN; + + assert.deepStrictEqual( + { + classicDefault: compositeBarLayoutHeight(false, false), + classicCompact: compositeBarLayoutHeight(true, false), + modernDefault: compositeBarLayoutHeight(false, true), + modernCompact: compositeBarLayoutHeight(true, true), + }, + { + // No floating gutters and no gap between items. + classicDefault: 300, + classicCompact: 300, + // Floating reserves a bottom gutter; the 8px gap is then handed back. + modernDefault: 300 - margin + ActivitybarPart.FLOATING_ACTION_GAP, + modernCompact: 300 - margin, + } + ); + }); + + // --- global activity icons reservation ----------------------------------- + + // The global (Accounts/Manage) icons are a separate action bar stacked beneath the view + // containers, so the room they take has to be measured rather than derived from the item + // size: the gap sits only *between* items, so N icons occupy N * height + (N - 1) * gap. + function heightLeftForCompositeBar(globalActionCount: number, itemHeight: number, gap: number): number { + const globalBarElement = document.createElement('div'); + for (let i = 0; i < globalActionCount; i++) { + const item = document.createElement('div'); + item.style.height = `${itemHeight}px`; + if (i > 0) { + item.style.marginTop = `${gap}px`; + } + globalBarElement.appendChild(item); + } + fixture.appendChild(globalBarElement); + + let laidOut: Dimension | undefined; + activityBarCompositeBarLayout.call({ + menuBarContainer: undefined, + globalCompositeBar: { element: globalBarElement }, + options: { orientation: ActionsOrientation.VERTICAL }, + compositeBar: { layout: dimension => { laidOut = dimension; } }, + }, ActivitybarPart.FLOATING_ACTIVITYBAR_WIDTH, 300); + + return laidOut!.height; + } + + test('reserves the measured height of the global activity icons', () => { + const gap = ActivitybarPart.FLOATING_ACTION_GAP; + const itemHeight = ActivitybarPart.FLOATING_ACTION_HEIGHT; + + assert.deepStrictEqual( + { + oneGlobalAction: heightLeftForCompositeBar(1, itemHeight, gap), + twoGlobalActions: heightLeftForCompositeBar(2, itemHeight, gap), + }, + { + // A lone icon has no gap at all, so it occupies exactly its own height. + oneGlobalAction: 300 - itemHeight, + // Two icons share a single gap: 36 + 8 + 36 = 80, not 2 * (36 + 8) = 88. + twoGlobalActions: 300 - (itemHeight * 2 + gap), + } + ); + }); + ensureNoDisposablesAreLeakedInTestSuite(); });