Merge pull request microsoft#167277 from microsoft/joh/confident-kite

move max menu/toolbar size into Toolbar itself, hidden item don't cou…

Author: jrieken

src/vs/platform/actions/browser/menuEntryActionViewItem.ts

@@ -37,19 +37,18 @@ export function createAndFillInContextMenuActions(menu: IMenu, options: IMenuAct
 	fillInActions(groups, target, useAlternativeActions, primaryGroup ? actionGroup => actionGroup === primaryGroup : actionGroup => actionGroup === 'navigation');
 }
 
-export function createAndFillInActionBarActions(menu: IMenu, options: IMenuActionOptions | undefined, target: IAction[] | { primary: IAction[]; secondary: IAction[] }, primaryGroup?: string | ((actionGroup: string) => boolean), primaryMaxCount?: number, shouldInlineSubmenu?: (action: SubmenuAction, group: string, groupSize: number) => boolean, useSeparatorsInPrimaryActions?: boolean): void {
+export function createAndFillInActionBarActions(menu: IMenu, options: IMenuActionOptions | undefined, target: IAction[] | { primary: IAction[]; secondary: IAction[] }, primaryGroup?: string | ((actionGroup: string) => boolean), shouldInlineSubmenu?: (action: SubmenuAction, group: string, groupSize: number) => boolean, useSeparatorsInPrimaryActions?: boolean): void {
 	const groups = menu.getActions(options);
 	const isPrimaryAction = typeof primaryGroup === 'string' ? (actionGroup: string) => actionGroup === primaryGroup : primaryGroup;
 
 	// Action bars handle alternative actions on their own so the alternative actions should be ignored
-	fillInActions(groups, target, false, isPrimaryAction, primaryMaxCount, shouldInlineSubmenu, useSeparatorsInPrimaryActions);
+	fillInActions(groups, target, false, isPrimaryAction, shouldInlineSubmenu, useSeparatorsInPrimaryActions);
 }
 
 function fillInActions(
 	groups: ReadonlyArray<[string, ReadonlyArray<MenuItemAction | SubmenuItemAction>]>, target: IAction[] | { primary: IAction[]; secondary: IAction[] },
 	useAlternativeActions: boolean,
 	isPrimaryAction: (actionGroup: string) => boolean = actionGroup => actionGroup === 'navigation',
-	primaryMaxCount: number = Number.MAX_SAFE_INTEGER,
 	shouldInlineSubmenu: (action: SubmenuAction, group: string, groupSize: number) => boolean = () => false,
 	useSeparatorsInPrimaryActions: boolean = false
 ): void {
@@ -101,16 +100,10 @@ function fillInActions(
 		// inlining submenus with length 0 or 1 is easy,
 		// larger submenus need to be checked with the overall limit
 		const submenuActions = action.actions;
-		if ((submenuActions.length <= 1 || target.length + submenuActions.length - 2 <= primaryMaxCount) && shouldInlineSubmenu(action, group, target.length)) {
+		if (submenuActions.length <= 1 && shouldInlineSubmenu(action, group, target.length)) {
 			target.splice(index, 1, ...submenuActions);
 		}
 	}
-
-	// overflow items from the primary group into the secondary bucket
-	if (primaryBucket !== secondaryBucket && primaryBucket.length > primaryMaxCount) {
-		const overflow = primaryBucket.splice(primaryMaxCount, primaryBucket.length - primaryMaxCount);
-		secondaryBucket.unshift(...overflow, new Separator());
-	}
 }
 
 export interface IMenuEntryActionViewItemOptions {

src/vs/platform/actions/browser/toolbar.ts

@@ -59,6 +59,11 @@ export type IWorkbenchToolBarOptions = IToolBarOptions & {
 
 	/** This is controlled by the WorkbenchToolBar */
 	allowContextMenu?: never;
+
+	/**
+	 * Maximun number of items that can shown. Extra items will be shown in the overflow menu.
+	 */
+	maxNumberOfItems?: number;
 };
 
 /**
@@ -107,10 +112,11 @@ export class WorkbenchToolBar extends ToolBar {
 		const secondary = _secondary.slice();
 		const toggleActions: IAction[] = [];
 
+		const extraSecondary: IAction[] = [];
+
 		let someAreHidden = false;
 		// unless disabled, move all hidden items to secondary group or ignore them
 		if (this._options?.hiddenItemStrategy !== HiddenItemStrategy.NoHide) {
-			let shouldPrependSeparator = secondary.length > 0;
 			for (let i = 0; i < primary.length; i++) {
 				const action = primary[i];
 				if (!(action instanceof MenuItemAction) && !(action instanceof SubmenuItemAction)) {
@@ -129,17 +135,30 @@ export class WorkbenchToolBar extends ToolBar {
 					someAreHidden = true;
 					primary[i] = undefined!;
 					if (this._options?.hiddenItemStrategy !== HiddenItemStrategy.Ignore) {
-						if (shouldPrependSeparator) {
-							shouldPrependSeparator = false;
-							secondary.unshift(new Separator());
-						}
-						secondary.unshift(action);
+						extraSecondary[i] = action;
 					}
 				}
 			}
 		}
+
+		// count for max
+		if (this._options?.maxNumberOfItems !== undefined) {
+			let count = 0;
+			for (let i = 0; i < primary.length; i++) {
+				const action = primary[i];
+				if (!action) {
+					continue;
+				}
+				if (++count >= this._options.maxNumberOfItems) {
+					primary[i] = undefined!;
+					extraSecondary[i] = action;
+				}
+			}
+		}
+
 		coalesceInPlace(primary);
-		super.setActions(primary, secondary);
+		coalesceInPlace(extraSecondary);
+		super.setActions(primary, Separator.join(extraSecondary, secondary));
 
 		// add context menu for toggle actions
 		if (toggleActions.length > 0) {
@@ -210,12 +229,6 @@ export interface IToolBarRenderOptions {
 	 */
 	primaryGroup?: string | ((actionGroup: string) => boolean);
 
-	/**
-	 * Limits the number of items that make it in the primary group. The rest overflows into the
-	 * secondary menu.
-	 */
-	primaryMaxCount?: number;
-
 	/**
 	 * Inlinse submenus with just a single item
 	 */
@@ -269,7 +282,7 @@ export class MenuWorkbenchToolBar extends WorkbenchToolBar {
 				menu,
 				options?.menuOptions,
 				{ primary, secondary },
-				options?.toolbarOptions?.primaryGroup, options?.toolbarOptions?.primaryMaxCount, options?.toolbarOptions?.shouldInlineSubmenu, options?.toolbarOptions?.useSeparatorsInPrimaryActions
+				options?.toolbarOptions?.primaryGroup, options?.toolbarOptions?.shouldInlineSubmenu, options?.toolbarOptions?.useSeparatorsInPrimaryActions
 			);
 			super.setActions(primary, secondary);
 		};

src/vs/workbench/browser/parts/editor/titleControl.ts

@@ -188,7 +188,8 @@ export abstract class TitleControl extends Themable {
 			anchorAlignmentProvider: () => AnchorAlignment.RIGHT,
 			renderDropdownAsChildElement: this.renderDropdownAsChildElement,
 			telemetrySource: 'editorPart',
-			resetMenu: MenuId.EditorTitle
+			resetMenu: MenuId.EditorTitle,
+			maxNumberOfItems: 9
 		}));
 
 		// Context
@@ -270,7 +271,6 @@ export abstract class TitleControl extends Themable {
 				{ arg: this.resourceContext.get(), shouldForwardArgs: true },
 				{ primary, secondary },
 				'navigation',
-				9,
 				shouldInlineGroup
 			);
 		}