polish from feedback

prateek · Jan 23, 2018 · b9cdcec · b9cdcec
1 parent 5974501
commit b9cdcec
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 31 deletions.
diff --git a/src/vs/code/electron-main/window.ts b/src/vs/code/electron-main/window.ts
@@ -953,7 +953,7 @@ export class CodeWindow implements ICodeWindow {
 		const segments: ITouchBarSegment[] = items.map(item => {
 			let icon: Electron.NativeImage;
 			if (item.iconPath) {
-				icon = nativeImage.createFromPath(typeof item.iconPath === 'string' ? item.iconPath : item.iconPath.dark);
+				icon = nativeImage.createFromPath(item.iconPath.dark);
 				if (icon.isEmpty()) {
 					icon = void 0;
 				}

diff --git a/src/vs/platform/actions/browser/menuItemActionItem.ts b/src/vs/platform/actions/browser/menuItemActionItem.ts
@@ -127,12 +127,12 @@ export class MenuItemActionItem extends ActionItem {
 	private _itemClassDispose: IDisposable;
 
 	constructor(
-		private action: MenuItemAction,
+		public _action: MenuItemAction,
 		@IKeybindingService private _keybindingService: IKeybindingService,
 		@IMessageService protected _messageService: IMessageService,
 		@IContextMenuService private _contextMenuService: IContextMenuService
 	) {
-		super(undefined, action, { icon: !!(action.class || action.item.iconPath), label: !action.class && !action.item.iconPath });
+		super(undefined, _action, { icon: !!(_action.class || _action.item.iconPath), label: !_action.class && !_action.item.iconPath });
 	}
 
 	protected get _commandAction(): IAction {
@@ -150,7 +150,7 @@ export class MenuItemActionItem extends ActionItem {
 	render(container: HTMLElement): void {
 		super.render(container);
 
-		this._updateItemClass(this.action.item);
+		this._updateItemClass(this._action.item);
 
 		let mouseOver = false;
 		let altDown = false;
@@ -200,9 +200,9 @@ export class MenuItemActionItem extends ActionItem {
 	_updateClass(): void {
 		if (this.options.icon) {
 			if (this._commandAction !== this._action) {
-				this._updateItemClass(this.action.alt.item);
+				this._updateItemClass(this._action.alt.item);
 			} else if ((<MenuItemAction>this._action).alt) {
-				this._updateItemClass(this.action.item);
+				this._updateItemClass(this._action.item);
 			}
 		}
 	}
@@ -213,23 +213,14 @@ export class MenuItemActionItem extends ActionItem {
 
 		if (item.iconPath) {
 			let iconClass: string;
-			if (typeof item.iconPath === 'string') {
-				if (MenuItemActionItem.ICON_PATH_TO_CSS_RULES.has(item.iconPath)) {
-					iconClass = MenuItemActionItem.ICON_PATH_TO_CSS_RULES.get(item.iconPath);
-				} else {
-					iconClass = ids.nextId();
-					createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath).toString()}")`);
-					MenuItemActionItem.ICON_PATH_TO_CSS_RULES.set(item.iconPath, iconClass);
-				}
+
+			if (MenuItemActionItem.ICON_PATH_TO_CSS_RULES.has(item.iconPath.dark)) {
+				iconClass = MenuItemActionItem.ICON_PATH_TO_CSS_RULES.get(item.iconPath.dark);
 			} else {
-				if (MenuItemActionItem.ICON_PATH_TO_CSS_RULES.has(item.iconPath.dark)) {
-					iconClass = MenuItemActionItem.ICON_PATH_TO_CSS_RULES.get(item.iconPath.dark);
-				} else {
-					iconClass = ids.nextId();
-					createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath.light).toString()}")`);
-					createCSSRule(`.vs-dark .icon.${iconClass}, .hc-black .icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath.dark).toString()}")`);
-					MenuItemActionItem.ICON_PATH_TO_CSS_RULES.set(item.iconPath.dark, iconClass);
-				}
+				iconClass = ids.nextId();
+				createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath.light || item.iconPath.dark).toString()}")`);
+				createCSSRule(`.vs-dark .icon.${iconClass}, .hc-black .icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath.dark).toString()}")`);
+				MenuItemActionItem.ICON_PATH_TO_CSS_RULES.set(item.iconPath.dark, iconClass);
 			}
 
 			this.$e.getHTMLElement().classList.add('icon', iconClass);

diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts
@@ -23,7 +23,7 @@ export interface ICommandAction {
 	id: string;
 	title: string | ILocalizedString;
 	category?: string | ILocalizedString;
-	iconPath?: string | { light: string; dark: string; };
+	iconPath?: { dark: string; light?: string; };
 	precondition?: ContextKeyExpr;
 }
 

diff --git a/src/vs/platform/actions/electron-browser/menusExtensionPoint.ts b/src/vs/platform/actions/electron-browser/menusExtensionPoint.ts
@@ -286,10 +286,10 @@ ExtensionsRegistry.registerExtensionPoint<schema.IUserFriendlyCommand | schema.I
 
 		const { icon, category, title, command } = userFriendlyCommand;
 
-		let absoluteIcon: string | { light: string; dark: string; };
+		let absoluteIcon: { dark: string; light?: string; };
 		if (icon) {
 			if (typeof icon === 'string') {
-				absoluteIcon = join(extension.description.extensionFolderPath, icon);
+				absoluteIcon = { dark: join(extension.description.extensionFolderPath, icon) };
 			} else {
 				absoluteIcon = {
 					dark: join(extension.description.extensionFolderPath, icon.dark),

diff --git a/src/vs/workbench/browser/parts/editor/editor.contribution.ts b/src/vs/workbench/browser/parts/editor/editor.contribution.ts
@@ -403,12 +403,12 @@ editorCommands.setup();
 // Touch Bar
 if (isMacintosh) {
 	MenuRegistry.appendMenuItem(MenuId.TouchBarContext, {
-		command: { id: NavigateBackwardsAction.ID, title: NavigateBackwardsAction.LABEL, iconPath: URI.parse(require.toUrl('vs/workbench/browser/parts/editor/media/back-tb.png')).fsPath },
+		command: { id: NavigateBackwardsAction.ID, title: NavigateBackwardsAction.LABEL, iconPath: { dark: URI.parse(require.toUrl('vs/workbench/browser/parts/editor/media/back-tb.png')).fsPath } },
 		group: 'navigation'
 	});
 
 	MenuRegistry.appendMenuItem(MenuId.TouchBarContext, {
-		command: { id: NavigateForwardAction.ID, title: NavigateForwardAction.LABEL, iconPath: URI.parse(require.toUrl('vs/workbench/browser/parts/editor/media/forward-tb.png')).fsPath },
+		command: { id: NavigateForwardAction.ID, title: NavigateForwardAction.LABEL, iconPath: { dark: URI.parse(require.toUrl('vs/workbench/browser/parts/editor/media/forward-tb.png')).fsPath } },
 		group: 'navigation'
 	});
 }

diff --git a/src/vs/workbench/parts/codeEditor/electron-browser/toggleWordWrap.ts b/src/vs/workbench/parts/codeEditor/electron-browser/toggleWordWrap.ts
@@ -258,7 +258,7 @@ MenuRegistry.appendMenuItem(MenuId.EditorTitle, {
 	command: {
 		id: 'editor.action.toggleWordWrap',
 		title: nls.localize('unwrapMinified', "Disable wrapping for this file"),
-		iconPath: URI.parse(require.toUrl('vs/workbench/parts/codeEditor/electron-browser/WordWrap_16x.svg')).fsPath
+		iconPath: { dark: URI.parse(require.toUrl('vs/workbench/parts/codeEditor/electron-browser/WordWrap_16x.svg')).fsPath }
 	},
 	group: 'navigation',
 	order: 1,
@@ -272,7 +272,7 @@ MenuRegistry.appendMenuItem(MenuId.EditorTitle, {
 	command: {
 		id: 'editor.action.toggleWordWrap',
 		title: nls.localize('wrapMinified', "Enable wrapping for this file"),
-		iconPath: URI.parse(require.toUrl('vs/workbench/parts/codeEditor/electron-browser/WordWrap_16x.svg')).fsPath
+		iconPath: { dark: URI.parse(require.toUrl('vs/workbench/parts/codeEditor/electron-browser/WordWrap_16x.svg')).fsPath }
 	},
 	group: 'navigation',
 	order: 1,

diff --git a/src/vs/workbench/parts/debug/electron-browser/debug.contribution.ts b/src/vs/workbench/parts/debug/electron-browser/debug.contribution.ts
@@ -223,7 +223,7 @@ if (isMacintosh) {
 	const registerTouchBarEntry = (id: string, title: string, order, when: ContextKeyExpr, icon: string) => {
 		MenuRegistry.appendMenuItem(MenuId.TouchBarContext, {
 			command: {
-				id, title, iconPath: URI.parse(require.toUrl(`vs/workbench/parts/debug/electron-browser/media/${icon}`)).fsPath
+				id, title, iconPath: { dark: URI.parse(require.toUrl(`vs/workbench/parts/debug/electron-browser/media/${icon}`)).fsPath }
 			},
 			when,
 			group: '9_debug',

diff --git a/src/vs/workbench/parts/files/electron-browser/fileActions.contribution.ts b/src/vs/workbench/parts/files/electron-browser/fileActions.contribution.ts
@@ -125,7 +125,7 @@ appendSaveConflictEditorTitleAction('workbench.files.action.revertLocalChanges',
 	dark: URI.parse(require.toUrl(`vs/workbench/parts/files/electron-browser/media/undo-inverse.svg`)).fsPath
 }, -9, revertLocalChangesCommand);
 
-function appendSaveConflictEditorTitleAction(id: string, title: string, iconPath: { dark: string; light: string; }, order: number, command: ICommandHandler): void {
+function appendSaveConflictEditorTitleAction(id: string, title: string, iconPath: { dark: string; light?: string; }, order: number, command: ICommandHandler): void {
 
 	// Command
 	CommandsRegistry.registerCommand(id, command);