UX Changes to ignore recommendations (#52514)

* UX Changes to ignore recommendations

* Fix missed negation when changing to shouldRecommend

* Update restored recommendation wording

* Update wording again

* Remove unused field

src/vs/platform/extensionManagement/common/extensionManagement.ts

@@ -378,11 +378,6 @@ export interface IExtensionEnablementService {
 	setEnablement(extension: ILocalExtension, state: EnablementState): TPromise<boolean>;
 }
 
-export interface IIgnoredRecommendations {
-	global: string[];
-	workspace: string[];
-}
-
 export interface IExtensionsConfigContent {
 	recommendations: string[];
 	unwantedRecommendations: string[];
@@ -416,8 +411,7 @@ export interface IExtensionTipsService {
 	getAllRecommendations(): TPromise<IExtensionRecommendation[]>;
 	getKeywordsForExtension(extension: string): string[];
 	getRecommendationsForExtension(extension: string): string[];
-	getAllIgnoredRecommendations(): IIgnoredRecommendations;
-	ignoreExtensionRecommendation(extensionId: string): void;
+	toggleIgnoredRecommendation(extensionId: string, shouldIgnore: boolean): void;
 	onRecommendationChange: Event<RecommendationChangeNotification>;
 }
 

src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts

@@ -31,7 +31,7 @@ import { Renderer, DataSource, Controller } from 'vs/workbench/parts/extensions/
 import { RatingsWidget, InstallCountWidget } from 'vs/workbench/parts/extensions/browser/extensionsWidgets';
 import { EditorOptions } from 'vs/workbench/common/editor';
 import { ActionBar } from 'vs/base/browser/ui/actionbar/actionbar';
-import { CombinedInstallAction, UpdateAction, EnableAction, DisableAction, ReloadAction, MaliciousStatusLabelAction, DisabledStatusLabelAction, MultiServerInstallAction, MultiServerUpdateAction, IgnoreExtensionRecommendationAction } from 'vs/workbench/parts/extensions/electron-browser/extensionsActions';
+import { CombinedInstallAction, UpdateAction, EnableAction, DisableAction, ReloadAction, MaliciousStatusLabelAction, DisabledStatusLabelAction, MultiServerInstallAction, MultiServerUpdateAction, IgnoreExtensionRecommendationAction, UndoIgnoreExtensionRecommendationAction } from 'vs/workbench/parts/extensions/electron-browser/extensionsActions';
 import { WebviewElement } from 'vs/workbench/parts/webview/electron-browser/webviewElement';
 import { KeybindingIO } from 'vs/workbench/services/keybinding/common/keybindingIO';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
@@ -389,19 +389,30 @@ export class ExtensionEditor extends BaseEditor {
 		this.transientDisposables.push(enableAction, updateAction, reloadAction, disableAction, installAction, maliciousStatusAction, disabledStatusAction);
 
 		const ignoreAction = this.instantiationService.createInstance(IgnoreExtensionRecommendationAction);
+		const undoIgnoreAction = this.instantiationService.createInstance(UndoIgnoreExtensionRecommendationAction);
 		ignoreAction.extension = extension;
+		undoIgnoreAction.extension = extension;
 
 		this.extensionTipsService.onRecommendationChange(change => {
-			if (change.extensionId.toLowerCase() === extension.id.toLowerCase() && change.isRecommended === false) {
-				addClass(this.header, 'recommendation-ignored');
-				removeClass(this.header, 'recommended');
-				this.recommendationText.textContent = localize('recommendationHasBeenIgnored', "You have chosen not to receive recommendations for this extension.");
+			if (change.extensionId.toLowerCase() === extension.id.toLowerCase()) {
+				if (change.isRecommended) {
+					const extRecommendations = this.extensionTipsService.getAllRecommendationsWithReason();
+					if (extRecommendations[extension.id.toLowerCase()]) {
+						removeClass(this.header, 'recommendation-ignored');
+						addClass(this.header, 'recommended');
+						this.recommendationText.textContent = extRecommendations[extension.id.toLowerCase()].reasonText;
+					}
+				} else {
+					addClass(this.header, 'recommendation-ignored');
+					removeClass(this.header, 'recommended');
+					this.recommendationText.textContent = localize('recommendationHasBeenIgnored', "You have chosen not to receive recommendations for this extension.");
+				}
 			}
 		});
 
 		this.ignoreActionbar.clear();
-		this.ignoreActionbar.push([ignoreAction], { icon: true, label: true });
-		this.transientDisposables.push(ignoreAction);
+		this.ignoreActionbar.push([ignoreAction, undoIgnoreAction], { icon: true, label: true });
+		this.transientDisposables.push(ignoreAction, undoIgnoreAction);
 
 		this.content.innerHTML = ''; // Clear content before setting navbar actions.
 

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts

@@ -10,7 +10,10 @@ import { forEach } from 'vs/base/common/collections';
 import { IDisposable, dispose, Disposable } from 'vs/base/common/lifecycle';
 import { match } from 'vs/base/common/glob';
 import * as json from 'vs/base/common/json';
-import { IExtensionManagementService, IExtensionGalleryService, IExtensionTipsService, ExtensionRecommendationReason, LocalExtensionType, EXTENSION_IDENTIFIER_PATTERN, IIgnoredRecommendations, IExtensionsConfigContent, RecommendationChangeNotification, IExtensionRecommendation, ExtensionRecommendationSource, IExtensionManagementServerService, InstallOperation } from 'vs/platform/extensionManagement/common/extensionManagement';
+import {
+	IExtensionManagementService, IExtensionGalleryService, IExtensionTipsService, ExtensionRecommendationReason, LocalExtensionType, EXTENSION_IDENTIFIER_PATTERN,
+	IExtensionsConfigContent, RecommendationChangeNotification, IExtensionRecommendation, ExtensionRecommendationSource, IExtensionManagementServerService, InstallOperation
+} from 'vs/platform/extensionManagement/common/extensionManagement';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { ITextModel } from 'vs/editor/common/model';
 import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
@@ -82,6 +85,8 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 
 	private readonly _onRecommendationChange: Emitter<RecommendationChangeNotification> = new Emitter<RecommendationChangeNotification>();
 	onRecommendationChange: Event<RecommendationChangeNotification> = this._onRecommendationChange.event;
+	private _sessionIgnoredRecommendations: { [id: string]: { reasonId: ExtensionRecommendationReason } } = {};
+	private _sessionRestoredRecommendations: { [id: string]: { reasonId: ExtensionRecommendationReason } } = {};
 
 	constructor(
 		@IExtensionGalleryService private readonly _galleryService: IExtensionGalleryService,
@@ -206,6 +211,11 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 			reasonText: localize('workspaceRecommendation', "This extension is recommended by users of the current workspace.")
 		});
 
+		Object.keys(this._sessionRestoredRecommendations).forEach(x => output[x.toLowerCase()] = {
+			reasonId: this._sessionRestoredRecommendations[x].reasonId,
+			reasonText: localize('restoredRecommendation', "You will receive recommendations for this extension in your next VS Code session.")
+		});
+
 		return output;
 	}
 
@@ -260,7 +270,6 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 
 					this._allIgnoredRecommendations = distinct([...this._globallyIgnoredRecommendations, ...this._workspaceIgnoredRecommendations]);
 					this.refilterAllRecommendations();
-
 				}));
 	}
 
@@ -358,13 +367,6 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 		}
 	}
 
-	getAllIgnoredRecommendations(): IIgnoredRecommendations {
-		return {
-			workspace: this._workspaceIgnoredRecommendations,
-			global: this._globallyIgnoredRecommendations
-		};
-	}
-
 	private isExtensionAllowedToBeRecommended(id: string): boolean {
 		return this._allIgnoredRecommendations.indexOf(id.toLowerCase()) === -1;
 	}
@@ -395,13 +397,14 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 					}
 				}
 				return this._fileBasedRecommendations[a].recommendedTime > this._fileBasedRecommendations[b].recommendedTime ? -1 : 1;
-			})
-			.map(extensionId => (<IExtensionRecommendation>{ extensionId, sources: this._fileBasedRecommendations[extensionId].sources }));
+			}).map(extensionId => (<IExtensionRecommendation>{ extensionId, sources: this._fileBasedRecommendations[extensionId].sources }));
 	}
 
 	getOtherRecommendations(): TPromise<IExtensionRecommendation[]> {
 		return this.fetchProactiveRecommendations().then(() => {
-			const others = distinct([...Object.keys(this._exeBasedRecommendations), ...this._dynamicWorkspaceRecommendations]);
+			const others = distinct([
+				...Object.keys(this._exeBasedRecommendations),
+				...this._dynamicWorkspaceRecommendations]);
 			shuffle(others);
 			return others.map(extensionId => {
 				const sources: ExtensionRecommendationSource[] = [];
@@ -960,29 +963,35 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 		return Object.keys(result);
 	}
 
-	ignoreExtensionRecommendation(extensionId: string): void {
-		/* __GDPR__
-			"extensionsRecommendations:ignoreRecommendation" : {
-				"recommendationReason": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
-				"extensionId": { "classification": "PublicNonPersonalData", "purpose": "FeatureInsight" }
+	toggleIgnoredRecommendation(extensionId: string, shouldIgnore: boolean): void {
+		const lowerId = extensionId.toLowerCase();
+		if (shouldIgnore) {
+			const reason = this.getAllRecommendationsWithReason()[lowerId];
+			if (reason && reason.reasonId) {
+				/* __GDPR__
+					"extensionsRecommendations:ignoreRecommendation" : {
+						"recommendationReason": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
+						"extensionId": { "classification": "PublicNonPersonalData", "purpose": "FeatureInsight" }
+					}
+				*/
+				this.telemetryService.publicLog('extensionsRecommendations:ignoreRecommendation', { id: extensionId, recommendationReason: reason.reasonId });
+			}
+			this._sessionIgnoredRecommendations[lowerId] = reason;
+			delete this._sessionRestoredRecommendations[lowerId];
+			this._globallyIgnoredRecommendations = distinct([...this._globallyIgnoredRecommendations, lowerId].map(id => id.toLowerCase()));
+		} else {
+			this._globallyIgnoredRecommendations = this._globallyIgnoredRecommendations.filter(id => id !== lowerId);
+			if (this._sessionIgnoredRecommendations[lowerId]) {
+				this._sessionRestoredRecommendations[lowerId] = this._sessionIgnoredRecommendations[lowerId];
+				delete this._sessionIgnoredRecommendations[lowerId];
 			}
-		*/
-		const reason = this.getAllRecommendationsWithReason()[extensionId.toLowerCase()];
-		if (reason && reason.reasonId) {
-			this.telemetryService.publicLog('extensionsRecommendations:ignoreRecommendation', { id: extensionId, recommendationReason: reason.reasonId });
 		}
-
-
-		this._globallyIgnoredRecommendations = distinct(
-			[...<string[]>JSON.parse(this.storageService.get('extensionsAssistant/ignored_recommendations', StorageScope.GLOBAL, '[]')), extensionId.toLowerCase()]
-				.map(id => id.toLowerCase()));
-
 		this.storageService.store('extensionsAssistant/ignored_recommendations', JSON.stringify(this._globallyIgnoredRecommendations), StorageScope.GLOBAL);
 
 		this._allIgnoredRecommendations = distinct([...this._globallyIgnoredRecommendations, ...this._workspaceIgnoredRecommendations]);
 
 		this.refilterAllRecommendations();
-		this._onRecommendationChange.fire({ extensionId: extensionId, isRecommended: false });
+		this._onRecommendationChange.fire({ extensionId: extensionId, isRecommended: !shouldIgnore });
 	}
 
 	dispose() {

src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts

@@ -1699,7 +1699,7 @@ export class IgnoreExtensionRecommendationAction extends Action {
 
 	static readonly ID = 'extensions.ignore';
 
-	private static readonly Class = 'extension-action ignore octicon octicon-x';
+	private static readonly Class = 'extension-action ignore';
 
 	private disposables: IDisposable[] = [];
 	extension: IExtension;
@@ -1707,7 +1707,7 @@ export class IgnoreExtensionRecommendationAction extends Action {
 	constructor(
 		@IExtensionTipsService private extensionsTipsService: IExtensionTipsService,
 	) {
-		super(IgnoreExtensionRecommendationAction.ID);
+		super(IgnoreExtensionRecommendationAction.ID, 'Ignore Recommendation');
 
 		this.class = IgnoreExtensionRecommendationAction.Class;
 		this.tooltip = localize('ignoreExtensionRecommendation', "Do not recommend this extension again");
@@ -1715,7 +1715,37 @@ export class IgnoreExtensionRecommendationAction extends Action {
 	}
 
 	public run(): TPromise<any> {
-		this.extensionsTipsService.ignoreExtensionRecommendation(this.extension.id);
+		this.extensionsTipsService.toggleIgnoredRecommendation(this.extension.id, true);
+		return TPromise.as(null);
+	}
+
+	dispose(): void {
+		super.dispose();
+		this.disposables = dispose(this.disposables);
+	}
+}
+
+export class UndoIgnoreExtensionRecommendationAction extends Action {
+
+	static readonly ID = 'extensions.ignore';
+
+	private static readonly Class = 'extension-action undo-ignore';
+
+	private disposables: IDisposable[] = [];
+	extension: IExtension;
+
+	constructor(
+		@IExtensionTipsService private extensionsTipsService: IExtensionTipsService,
+	) {
+		super(UndoIgnoreExtensionRecommendationAction.ID, 'Undo');
+
+		this.class = UndoIgnoreExtensionRecommendationAction.Class;
+		this.tooltip = localize('undo', "Undo");
+		this.enabled = true;
+	}
+
+	public run(): TPromise<any> {
+		this.extensionsTipsService.toggleIgnoredRecommendation(this.extension.id, false);
 		return TPromise.as(null);
 	}
 

src/vs/workbench/parts/extensions/electron-browser/extensionsList.ts

@@ -170,7 +170,7 @@ export class Renderer implements IPagedRenderer<IExtension, ITemplateData> {
 
 		this.updateRecommendationStatus(extension, data);
 		data.extensionDisposables.push(this.extensionTipsService.onRecommendationChange(change => {
-			if (change.extensionId.toLowerCase() === extension.id.toLowerCase() && change.isRecommended === false) {
+			if (change.extensionId.toLowerCase() === extension.id.toLowerCase()) {
 				this.updateRecommendationStatus(extension, data);
 			}
 		}));

src/vs/workbench/parts/extensions/electron-browser/media/extensionActions.css

@@ -76,24 +76,3 @@
 .extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .actions-container {
 	justify-content: flex-start;
 }
-
-.extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .action-item .action-label.extension-action.ignore {
-	height: 13px;
-	width: 8px;
-	border: none;
-	outline-offset: 0;
-	margin-left: 6px;
-	padding-left: 0;
-	margin-top: 3px;
-	background-color: transparent;
-	color: hsl(0, 66%, 50%);
-}
-
-.hc-black .extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .action-item .action-label.extension-action.ignore,
-.vs-dark .extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .action-item .action-label.extension-action.ignore {
-	color: hsla(0, 66%, 77%, 1);
-}
-
-.extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .action-item .action-label.extension-action.ignore:hover {
-	filter: brightness(.8);
-}
\ No newline at end of file

src/vs/workbench/parts/extensions/electron-browser/media/extensionEditor.css

@@ -14,8 +14,11 @@
 
 .extension-editor > .header {
 	display: flex;
-	height: 128px;
-	padding: 20px;
+	height: 134px;
+	padding-top: 20px;
+	padding-bottom: 14px;
+	padding-left: 20px;
+	padding-right: 20px;
 	overflow: hidden;
 	font-size: 14px;
 }
@@ -23,7 +26,7 @@
 .extension-editor > .header.recommended,
 .extension-editor > .header.recommendation-ignored
 {
-	height: 140px;
+	height: 146px;
 }
 
 .extension-editor > .header > .icon {
@@ -130,23 +133,47 @@
 	padding: 1px 6px;
 }
 
+.extension-editor > .header > .details > .recommendation {
+	display: none;
+}
 
 .extension-editor > .header.recommended > .details > .recommendation,
 .extension-editor > .header.recommendation-ignored > .details > .recommendation {
 	display: flex;
-	margin-top: 2px;
+	margin-top: 0;
 	font-size: 13px;
+	height: 25px;
+	font-style: italic;
 }
 
-.extension-editor > .header > .details > .recommendation {
+.extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar,
+.extension-editor > .header.recommendation-ignored > .details > .recommendation > .monaco-action-bar {
+	margin-left: 4px;
+	margin-top: 2px;
+	font-style: normal;
+}
+
+.extension-editor > .header > .details > .recommendation > .recommendation-text {
+	margin-top: 5px;
+}
+.extension-editor > .header > .details > .recommendation > .monaco-action-bar .action-label {
+	margin-top: 4px;
+	margin-left: 4px;
+}
+
+.extension-editor > .header.recommendation-ignored > .details > .recommendation > .monaco-action-bar .ignore {
 	display: none;
 }
 
-.extension-editor > .header.recommendation-ignored > .details > .recommendation > .recommendation-text {
-	font-style: italic;
+.extension-editor > .header.recommendation-ignored > .details > .recommendation > .monaco-action-bar .undo-ignore {
+	display: block;
 }
 
-.extension-editor > .header.recommendation-ignored > .details > .recommendation > .monaco-action-bar {
+.extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .ignore {
+	display: block;
+}
+
+.extension-editor > .header.recommended > .details > .recommendation > .monaco-action-bar .undo-ignore {
 	display: none;
 }
 

src/vs/workbench/parts/extensions/test/electron-browser/extensionsTipsService.test.ts

@@ -424,17 +424,16 @@ suite('ExtensionsTipsService Test', () => {
 		return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions, workspaceIgnoredRecommendations).then(() => {
 			testObject = instantiationService.createInstance(ExtensionTipsService);
 			return testObject.loadRecommendationsPromise.then(() => {
-				const recommendations = testObject.getAllIgnoredRecommendations();
-				assert.deepStrictEqual(recommendations,
-					{
-						global: ['mockpublisher2.mockextension2'],
-						workspace: ['ms-vscode.csharp']
-					});
+				const recommendations = testObject.getAllRecommendationsWithReason();
+				assert.ok(recommendations['ms-python.python']);
+
+				assert.ok(!recommendations['mockpublisher2.mockextension2']);
+				assert.ok(!recommendations['ms-vscode.csharp']);
 			});
 		});
 	});
 
-	test('ExtensionTipsService: Able to dynamically ignore global recommendations', () => {
+	test('ExtensionTipsService: Able to dynamically ignore/unignore global recommendations', () => {
 		const storageGetterStub = (a, _, c) => {
 			const storedRecommendations = '["ms-vscode.csharp", "ms-python.python"]';
 			const globallyIgnoredRecommendations = '["mockpublisher2.mockextension2"]'; // ignore a workspace recommendation.
@@ -452,20 +451,27 @@ suite('ExtensionsTipsService Test', () => {
 		return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => {
 			testObject = instantiationService.createInstance(ExtensionTipsService);
 			return testObject.loadRecommendationsPromise.then(() => {
-				const recommendations = testObject.getAllIgnoredRecommendations();
-				assert.deepStrictEqual(recommendations,
-					{
-						global: ['mockpublisher2.mockextension2'],
-						workspace: []
-					});
-				return testObject.ignoreExtensionRecommendation('mockpublisher1.mockextension1');
+				const recommendations = testObject.getAllRecommendationsWithReason();
+				assert.ok(recommendations['ms-python.python']);
+				assert.ok(recommendations['mockpublisher1.mockextension1']);
+
+				assert.ok(!recommendations['mockpublisher2.mockextension2']);
+
+				return testObject.toggleIgnoredRecommendation('mockpublisher1.mockextension1', true);
 			}).then(() => {
-				const recommendations = testObject.getAllIgnoredRecommendations();
-				assert.deepStrictEqual(recommendations,
-					{
-						global: ['mockpublisher2.mockextension2', 'mockpublisher1.mockextension1'],
-						workspace: []
-					});
+				const recommendations = testObject.getAllRecommendationsWithReason();
+				assert.ok(recommendations['ms-python.python']);
+
+				assert.ok(!recommendations['mockpublisher1.mockextension1']);
+				assert.ok(!recommendations['mockpublisher2.mockextension2']);
+
+				return testObject.toggleIgnoredRecommendation('mockpublisher1.mockextension1', false);
+			}).then(() => {
+				const recommendations = testObject.getAllRecommendationsWithReason();
+				assert.ok(recommendations['ms-python.python']);
+
+				assert.ok(recommendations['mockpublisher1.mockextension1']);
+				assert.ok(!recommendations['mockpublisher2.mockextension2']);
 			});
 		});
 	});
@@ -484,7 +490,7 @@ suite('ExtensionsTipsService Test', () => {
 		return setUpFolderWorkspace('myFolder', []).then(() => {
 			testObject = instantiationService.createInstance(ExtensionTipsService);
 			testObject.onRecommendationChange(changeHandlerTarget);
-			testObject.ignoreExtensionRecommendation(ignoredExtensionId);
+			testObject.toggleIgnoredRecommendation(ignoredExtensionId, true);
 
 			assert.ok(changeHandlerTarget.calledOnce);
 			assert.ok(changeHandlerTarget.getCall(0).calledWithMatch({ extensionId: 'Some.Extension', isRecommended: false }));