diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar.ts index 60e141d23df1e..56fbf1b593775 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar.ts @@ -88,7 +88,11 @@ class WorkbenchAlwaysLabelStrategy implements IActionLayoutStrategy { const initialPrimaryActions = this.editorToolbar.primaryActions; const initialSecondaryActions = this.editorToolbar.secondaryActions; - return workbenchCalculateActions(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth); + const actionOutput = workbenchCalculateActions(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth); + return { + primaryActions: actionOutput.primaryActions.map(a => a.action), + secondaryActions: actionOutput.secondaryActions + }; } } @@ -111,7 +115,11 @@ class WorkbenchNeverLabelStrategy implements IActionLayoutStrategy { const initialPrimaryActions = this.editorToolbar.primaryActions; const initialSecondaryActions = this.editorToolbar.secondaryActions; - return workbenchCalculateActions(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth); + const actionOutput = workbenchCalculateActions(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth); + return { + primaryActions: actionOutput.primaryActions.map(a => a.action), + secondaryActions: actionOutput.secondaryActions + }; } } @@ -139,7 +147,11 @@ class WorkbenchDynamicLabelStrategy implements IActionLayoutStrategy { const initialPrimaryActions = this.editorToolbar.primaryActions; const initialSecondaryActions = this.editorToolbar.secondaryActions; - return workbenchDynamicCalculateActions(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth); + const actionOutput = workbenchDynamicCalculateActions(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth); + return { + primaryActions: actionOutput.primaryActions.map(a => a.action), + secondaryActions: actionOutput.secondaryActions + }; } } @@ -523,11 +535,11 @@ export class NotebookEditorWorkbenchToolbar extends Disposable { } } -export function workbenchCalculateActions(initialPrimaryActions: IActionModel[], initialSecondaryActions: IAction[], leftToolbarContainerMaxWidth: number): { primaryActions: IAction[]; secondaryActions: IAction[] } { +export function workbenchCalculateActions(initialPrimaryActions: IActionModel[], initialSecondaryActions: IAction[], leftToolbarContainerMaxWidth: number): { primaryActions: IActionModel[]; secondaryActions: IAction[] } { return actionOverflowHelper(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth, false); } -export function workbenchDynamicCalculateActions(initialPrimaryActions: IActionModel[], initialSecondaryActions: IAction[], leftToolbarContainerMaxWidth: number): { primaryActions: IAction[]; secondaryActions: IAction[] } { +export function workbenchDynamicCalculateActions(initialPrimaryActions: IActionModel[], initialSecondaryActions: IAction[], leftToolbarContainerMaxWidth: number): { primaryActions: IActionModel[]; secondaryActions: IAction[] } { if (initialPrimaryActions.length === 0) { return { primaryActions: [], secondaryActions: initialSecondaryActions }; @@ -542,10 +554,7 @@ export function workbenchDynamicCalculateActions(initialPrimaryActions: IActionM initialPrimaryActions.forEach(action => { action.renderLabel = true; }); - return { - primaryActions: initialPrimaryActions.map(action => action.action), - secondaryActions: initialSecondaryActions - }; + return actionOverflowHelper(initialPrimaryActions, initialSecondaryActions, leftToolbarContainerMaxWidth, false); } // step 2: check if they fit without labels @@ -562,7 +571,7 @@ export function workbenchDynamicCalculateActions(initialPrimaryActions: IActionM if (initialPrimaryActions[i].action instanceof Separator) { // find group separator - const remainingItems = initialPrimaryActions.slice(i + 1); + const remainingItems = initialPrimaryActions.slice(i + 1).filter(action => action.size !== 0); // todo: need to exclude size 0 items from this const newTotalSum = sum + (remainingItems.length === 0 ? 0 : (remainingItems.length * ICON_ONLY_ACTION_WIDTH + (remainingItems.length - 1) * ACTION_PADDING)); if (newTotalSum <= leftToolbarContainerMaxWidth) { lastActionWithLabel = i; @@ -582,12 +591,12 @@ export function workbenchDynamicCalculateActions(initialPrimaryActions: IActionM initialPrimaryActions.slice(0, lastActionWithLabel + 1).forEach(action => { action.renderLabel = true; }); initialPrimaryActions.slice(lastActionWithLabel + 1).forEach(action => { action.renderLabel = false; }); return { - primaryActions: initialPrimaryActions.map(action => action.action), + primaryActions: initialPrimaryActions, secondaryActions: initialSecondaryActions }; } -function actionOverflowHelper(initialPrimaryActions: IActionModel[], initialSecondaryActions: IAction[], leftToolbarContainerMaxWidth: number, iconOnly: boolean): { primaryActions: IAction[]; secondaryActions: IAction[] } { +function actionOverflowHelper(initialPrimaryActions: IActionModel[], initialSecondaryActions: IAction[], leftToolbarContainerMaxWidth: number, iconOnly: boolean): { primaryActions: IActionModel[]; secondaryActions: IAction[] } { const renderActions: IActionModel[] = []; const overflow: IAction[] = []; @@ -665,7 +674,7 @@ function actionOverflowHelper(initialPrimaryActions: IActionModel[], initialSeco } return { - primaryActions: renderActions.map(action => action.action), + primaryActions: renderActions, secondaryActions: [...overflow, ...initialSecondaryActions] }; } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookWorkbenchToolbar.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookWorkbenchToolbar.test.ts index 4df6550371bf3..f36258029d7cc 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookWorkbenchToolbar.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookWorkbenchToolbar.test.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { workbenchCalculateActions } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar'; +import { workbenchCalculateActions, workbenchDynamicCalculateActions } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar'; import { Action, IAction, Separator } from 'vs/base/common/actions'; import * as assert from 'assert'; @@ -24,7 +24,7 @@ interface IActionModel { * * ex: action with size 50 requires 58px of space */ -suite('workbenchCalculateActions', () => { +suite('Workbench Toolbar calculateActions (strategy always + never)', () => { const defaultSecondaryActionModels: IActionModel[] = [ { action: new Action('secondaryAction0', 'Secondary Action 0'), size: 50, visible: true, renderLabel: true }, @@ -48,7 +48,7 @@ suite('workbenchCalculateActions', () => { { action: new Action('action2', 'Action 2'), size: 50, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 200); - assert.deepEqual(result.primaryActions, actions.map(action => action.action)); + assert.deepEqual(result.primaryActions, actions); assert.deepEqual(result.secondaryActions, defaultSecondaryActions); }); @@ -59,7 +59,7 @@ suite('workbenchCalculateActions', () => { { action: new Action('action2', 'Action 2'), size: 50, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 100); - assert.deepEqual(result.primaryActions, [actions[0]].map(action => action.action)); + assert.deepEqual(result.primaryActions, [actions[0]]); assert.deepEqual(result.secondaryActions, [actions[1], actions[2], separator, ...defaultSecondaryActionModels].map(action => action.action)); }); @@ -71,7 +71,7 @@ suite('workbenchCalculateActions', () => { { action: new Action('action1', 'Action 1'), size: 50, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 125); - assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[3]].map(action => action.action)); + assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[3]]); assert.deepEqual(result.secondaryActions, defaultSecondaryActions); }); @@ -83,7 +83,7 @@ suite('workbenchCalculateActions', () => { { action: new Separator(), size: 1, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 200); - assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[2]].map(action => action.action)); + assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[2]]); assert.deepEqual(result.secondaryActions, defaultSecondaryActions); }); @@ -95,7 +95,7 @@ suite('workbenchCalculateActions', () => { { action: new Action('action3', 'Action 3'), size: 0, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 116); - assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[3]].map(action => action.action)); + assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[3]]); assert.deepEqual(result.secondaryActions, [actions[2], separator, ...defaultSecondaryActionModels].map(action => action.action)); }); @@ -106,7 +106,7 @@ suite('workbenchCalculateActions', () => { { action: new Action('action1', 'Action 1'), size: 50, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 116); - assert.deepEqual(result.primaryActions, [actions[0], actions[2]].map(action => action.action)); + assert.deepEqual(result.primaryActions, [actions[0], actions[2]]); assert.deepEqual(result.secondaryActions, defaultSecondaryActions); }); @@ -120,7 +120,184 @@ suite('workbenchCalculateActions', () => { { action: new Action('action3', 'Action 3'), size: 50, visible: true, renderLabel: true }, ]; const result = workbenchCalculateActions(actions, defaultSecondaryActions, 300); - assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[2], actions[3], actions[5]].map(action => action.action)); + assert.deepEqual(result.primaryActions, [actions[0], actions[1], actions[2], actions[3], actions[5]]); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); +}); + +suite('Workbench Toolbar Dynamic calculateActions (strategy dynamic)', () => { + + const actionTemplate = [ + new Action('action0', 'Action 0'), + new Action('action1', 'Action 1'), + new Action('action2', 'Action 2'), + new Action('action3', 'Action 3') + ]; + + const defaultSecondaryActionModels: IActionModel[] = [ + { action: new Action('secondaryAction0', 'Secondary Action 0'), size: 50, visible: true, renderLabel: true }, + { action: new Action('secondaryAction1', 'Secondary Action 1'), size: 50, visible: true, renderLabel: true }, + { action: new Action('secondaryAction2', 'Secondary Action 2'), size: 50, visible: true, renderLabel: true }, + ]; + const defaultSecondaryActions: IAction[] = defaultSecondaryActionModels.map(action => action.action); + + + test('should return empty primary and secondary actions when given empty initial actions', () => { + const result = workbenchDynamicCalculateActions([], [], 100); + assert.deepEqual(result.primaryActions, []); + assert.deepEqual(result.secondaryActions, []); + }); + + test('should return all primary actions as visiblewhen they fit within the container width', () => { + const constainerSize = 200; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: true }, + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: true }, + ]; + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, constainerSize); + assert.deepEqual(result.primaryActions, expected); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); + + test('actions all within a group that cannot all fit, will all be icon only', () => { + const containerSize = 150; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: true }, + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: false }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: false }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: false }, + ]; + + + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expected); + assert.deepEqual(result.secondaryActions, [...defaultSecondaryActionModels].map(action => action.action)); + }); + + test('should ignore second separator when two separators are in a row', () => { + const containerSize = 200; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + ]; + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expected); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); + + test('check label visibility in different groupings', () => { + const containerSize = 150; + const actions: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: true }, + ]; + const expectedOutputActions: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: false }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: false }, + ]; + + + const result = workbenchDynamicCalculateActions(actions, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expectedOutputActions); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); + + test('should ignore separators when they are at the end of the resulting primary actions', () => { + const containerSize = 200; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + ]; + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expected); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); + + test('should keep actions with size 0 in primary actions', () => { + const containerSize = 170; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[3], size: 0, visible: true, renderLabel: true }, + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 50, visible: true, renderLabel: false }, + { action: actionTemplate[3], size: 0, visible: true, renderLabel: false }, + ]; + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expected); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); + + test('should not render separator if preceeded by size 0 action(s), but keep size 0 action in primary.', () => { + const containerSize = 116; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 0, visible: true, renderLabel: true }, // hidden + { action: new Separator(), size: 1, visible: true, renderLabel: true }, // sep + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true }, // visible + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 0, visible: true, renderLabel: true }, // hidden + { action: actionTemplate[1], size: 50, visible: true, renderLabel: true } // visible + ]; + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expected); + assert.deepEqual(result.secondaryActions, defaultSecondaryActions); + }); + + test('should not render second separator if space between is hidden (size 0) actions.', () => { + const containerSize = 300; + const input: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 0, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 0, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[3], size: 50, visible: true, renderLabel: true }, + ]; + const expected: IActionModel[] = [ + { action: actionTemplate[0], size: 50, visible: true, renderLabel: true }, + { action: new Separator(), size: 1, visible: true, renderLabel: true }, + { action: actionTemplate[1], size: 0, visible: true, renderLabel: true }, + { action: actionTemplate[2], size: 0, visible: true, renderLabel: true }, + // remove separator here + { action: actionTemplate[3], size: 50, visible: true, renderLabel: true }, + ]; + const result = workbenchDynamicCalculateActions(input, defaultSecondaryActions, containerSize); + assert.deepEqual(result.primaryActions, expected); assert.deepEqual(result.secondaryActions, defaultSecondaryActions); }); });