Skip to content

Commit 17ed228

Browse files
author
Benjamin Pasero
committed
debt - make notification#prompt() nicer to use
1 parent 62bc914 commit 17ed228

32 files changed

Lines changed: 672 additions & 600 deletions

File tree

‎src/vs/editor/standalone/browser/simpleServices.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKe
3838
import { OS } from 'vs/base/common/platform';
3939
import { IRange } from 'vs/editor/common/core/range';
4040
import { ITextModel } from 'vs/editor/common/model';
41-
import { INotificationService, INotification, INotificationHandle, NoOpNotification, PromptOption } from 'vs/platform/notification/common/notification';
41+
import { INotificationService, INotification, INotificationHandle, NoOpNotification, IPromptChoice } from 'vs/platform/notification/common/notification';
4242
import { IConfirmation, IConfirmationResult, IDialogService, IDialogOptions } from 'vs/platform/dialogs/common/dialogs';
4343
import { IPosition, Position as Pos } from 'vs/editor/common/core/position';
4444

@@ -297,8 +297,8 @@ export class SimpleNotificationService implements INotificationService {
297297
return SimpleNotificationService.NO_OP;
298298
}
299299

300-
public prompt(severity: Severity, message: string, choices: PromptOption[]): TPromise<number> {
301-
return TPromise.as(0);
300+
public prompt(severity: Severity, message: string, choices: IPromptChoice[], onCancel?: () => void): INotificationHandle {
301+
return SimpleNotificationService.NO_OP;
302302
}
303303
}
304304

‎src/vs/platform/integrity/node/integrityServiceImpl.ts‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import URI from 'vs/base/common/uri';
1414
import Severity from 'vs/base/common/severity';
1515
import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
1616
import { ILifecycleService, LifecyclePhase } from 'vs/platform/lifecycle/common/lifecycle';
17-
import { INotificationService, PromptOption } from 'vs/platform/notification/common/notification';
17+
import { INotificationService } from 'vs/platform/notification/common/notification';
1818

1919
interface IStorageData {
2020
dontShowPrompt: boolean;
@@ -82,26 +82,24 @@ export class IntegrityServiceImpl implements IIntegrityService {
8282
private _prompt(): void {
8383
const storedData = this._storage.get();
8484
if (storedData && storedData.dontShowPrompt && storedData.commit === product.commit) {
85-
// Do not prompt
86-
return;
85+
return; // Do not prompt
8786
}
8887

89-
const choices: PromptOption[] = [nls.localize('integrity.moreInformation', "More Information"), { label: nls.localize('integrity.dontShowAgain', "Don't Show Again") }];
90-
91-
this.notificationService.prompt(Severity.Warning, nls.localize('integrity.prompt', "Your {0} installation appears to be corrupt. Please reinstall.", product.nameShort), choices).then(choice => {
92-
switch (choice) {
93-
case 0 /* More Information */:
94-
const uri = URI.parse(product.checksumFailMoreInfoUrl);
95-
window.open(uri.toString(true));
96-
break;
97-
case 1 /* Do not show again */:
98-
this._storage.set({
99-
dontShowPrompt: true,
100-
commit: product.commit
101-
});
102-
break;
103-
}
104-
});
88+
this.notificationService.prompt(
89+
Severity.Warning,
90+
nls.localize('integrity.prompt', "Your {0} installation appears to be corrupt. Please reinstall.", product.nameShort),
91+
[
92+
{
93+
label: nls.localize('integrity.moreInformation', "More Information"),
94+
run: () => window.open(URI.parse(product.checksumFailMoreInfoUrl).toString(true))
95+
},
96+
{
97+
label: nls.localize('integrity.dontShowAgain', "Don't Show Again"),
98+
isSecondary: true,
99+
run: () => this._storage.set({ dontShowPrompt: true, commit: product.commit })
100+
}
101+
]
102+
);
105103
}
106104

107105
public isPure(): Thenable<IntegrityTestResult> {

‎src/vs/platform/keybinding/test/common/abstractKeybindingService.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKe
1919
import { OS } from 'vs/base/common/platform';
2020
import { IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
2121
import { NullTelemetryService } from 'vs/platform/telemetry/common/telemetryUtils';
22-
import { INotificationService, NoOpNotification, INotification } from 'vs/platform/notification/common/notification';
22+
import { INotificationService, NoOpNotification, INotification, IPromptChoice } from 'vs/platform/notification/common/notification';
2323

2424
function createContext(ctx: any) {
2525
return {
@@ -139,8 +139,8 @@ suite('AbstractKeybindingService', () => {
139139
showMessageCalls.push({ sev: Severity.Error, message });
140140
return new NoOpNotification();
141141
},
142-
prompt: () => {
143-
return TPromise.as(0);
142+
prompt(severity: Severity, message: string, choices: IPromptChoice[], onCancel?: () => void) {
143+
throw new Error('not implemented');
144144
}
145145
};
146146

‎src/vs/platform/notification/common/notification.ts‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation'
1010
import { IDisposable } from 'vs/base/common/lifecycle';
1111
import { IAction } from 'vs/base/common/actions';
1212
import { Event, Emitter } from 'vs/base/common/event';
13-
import { TPromise } from 'vs/base/common/winjs.base';
1413

1514
export import Severity = BaseSeverity;
1615

@@ -121,26 +120,30 @@ export interface INotificationHandle extends IDisposable {
121120
updateActions(actions?: INotificationActions): void;
122121
}
123122

123+
export interface IPromptChoice {
124124

125-
/**
126-
* Primary choices show up as buttons in the notification below the message.
127-
*/
128-
export type PrimaryPromptChoice = string;
129-
130-
/**
131-
* Secondary choices show up under the gear icon in the header of the notification.
132-
*/
133-
export interface SecondaryPromptChoice {
125+
/**
126+
* Label to show for the choice to the user.
127+
*/
134128
label: string;
135129

136130
/**
137-
* Wether to keep the notification open after the secondary choice was selected
131+
* Primary choices show up as buttons in the notification below the message.
132+
* Secondary choices show up under the gear icon in the header of the notification.
133+
*/
134+
isSecondary?: boolean;
135+
136+
/**
137+
* Wether to keep the notification open after the choice was selected
138138
* by the user. By default, will close the notification upon click.
139139
*/
140140
keepOpen?: boolean;
141-
}
142141

143-
export type PromptOption = PrimaryPromptChoice | SecondaryPromptChoice;
142+
/**
143+
* Triggered when the user selects the choice.
144+
*/
145+
run: () => void;
146+
}
144147

145148
/**
146149
* A service to bring up notifications and non-modal prompts.
@@ -185,10 +188,12 @@ export interface INotificationService {
185188
* Shows a prompt in the notification area with the provided choices. The prompt
186189
* is non-modal. If you want to show a modal dialog instead, use `IDialogService`.
187190
*
188-
* @returns a promise that will resolve to the index of the choice that was picked.
189-
* The promise can be cancelled to hide the notification prompt.
191+
* @param onCancel will be called if the user closed the notification without picking
192+
* any of the provided choices.
193+
*
194+
* @returns a handle on the notification to e.g. hide it or update message, buttons, etc.
190195
*/
191-
prompt(severity: Severity, message: string, choices: PromptOption[]): TPromise<number>;
196+
prompt(severity: Severity, message: string, choices: IPromptChoice[], onCancel?: () => void): INotificationHandle;
192197
}
193198

194199
export class NoOpNotification implements INotificationHandle {

‎src/vs/workbench/common/notifications.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ export class NotificationViewItem implements INotificationViewItem {
582582
}
583583

584584
for (let i = 0; i < primaryActions.length; i++) {
585-
if (primaryActions[i].id !== otherPrimaryActions[i].id) {
585+
if ((primaryActions[i].id + primaryActions[i].label) !== (otherPrimaryActions[i].id + otherPrimaryActions[i].label)) {
586586
return false;
587587
}
588588
}

‎src/vs/workbench/parts/extensions/browser/extensionsActions.ts‎

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { IAction, Action } from 'vs/base/common/actions';
1010
import { Throttler } from 'vs/base/common/async';
1111
import * as DOM from 'vs/base/browser/dom';
1212
import * as paths from 'vs/base/common/paths';
13-
import { Event, once } from 'vs/base/common/event';
13+
import { Event } from 'vs/base/common/event';
1414
import * as json from 'vs/base/common/json';
1515
import { ActionItem, IActionItem, Separator } from 'vs/base/browser/ui/actionbar/actionbar';
1616
import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
@@ -49,21 +49,23 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
4949
import { IQuickOpenService, IPickOpenEntry } from 'vs/platform/quickOpen/common/quickOpen';
5050

5151
const promptDownloadManually = (extension: IExtension, message: string, instantiationService: IInstantiationService, notificationService: INotificationService, openerService: IOpenerService) => {
52-
notificationService.prompt(Severity.Error, message, [localize('download', "Download Manually")]).done(choice => {
53-
if (choice === 0) {
54-
openerService.open(URI.parse(extension.downloadUrl)).then(() => {
55-
const action = instantiationService.createInstance(InstallVSIXAction, InstallVSIXAction.ID, InstallVSIXAction.LABEL);
56-
const handle = notificationService.notify({
57-
severity: Severity.Info,
58-
message: localize('install vsix', 'Once downloaded, please manually install the downloaded VSIX of \'{0}\'.', extension.id),
59-
actions: {
60-
primary: [action]
52+
notificationService.prompt(Severity.Error, message, [{
53+
label: localize('download', "Download Manually"),
54+
run: () => openerService.open(URI.parse(extension.downloadUrl)).then(() => {
55+
notificationService.prompt(
56+
Severity.Info,
57+
localize('install vsix', 'Once downloaded, please manually install the downloaded VSIX of \'{0}\'.', extension.id),
58+
[{
59+
label: InstallVSIXAction.LABEL,
60+
run: () => {
61+
const action = instantiationService.createInstance(InstallVSIXAction, InstallVSIXAction.ID, InstallVSIXAction.LABEL);
62+
action.run();
63+
action.dispose();
6164
}
62-
});
63-
once(handle.onDidDispose)(() => action.dispose());
64-
});
65-
}
66-
});
65+
}]
66+
);
67+
})
68+
}]);
6769
};
6870

6971
export class InstallAction extends Action {
@@ -1882,13 +1884,13 @@ export class InstallVSIXAction extends Action {
18821884
label = InstallVSIXAction.LABEL,
18831885
@IExtensionsWorkbenchService private extensionsWorkbenchService: IExtensionsWorkbenchService,
18841886
@INotificationService private notificationService: INotificationService,
1885-
@IWindowService private windowsService: IWindowService
1887+
@IWindowService private windowService: IWindowService
18861888
) {
18871889
super(id, label, 'extension-action install-vsix', true);
18881890
}
18891891

18901892
run(): TPromise<any> {
1891-
return this.windowsService.showOpenDialog({
1893+
return this.windowService.showOpenDialog({
18921894
title: localize('installFromVSIX', "Install from VSIX"),
18931895
filters: [{ name: 'VSIX Extensions', extensions: ['vsix'] }],
18941896
properties: ['openFile'],
@@ -1899,19 +1901,19 @@ export class InstallVSIXAction extends Action {
18991901
}
19001902

19011903
return TPromise.join(result.map(vsix => this.extensionsWorkbenchService.install(vsix))).then(() => {
1902-
return this.notificationService.prompt(Severity.Info, localize('InstallVSIXAction.success', "Successfully installed the extension. Reload to enable it."), [localize('InstallVSIXAction.reloadNow', "Reload Now")]).then(choice => {
1903-
if (choice === 0) {
1904-
return this.windowsService.reloadWindow();
1905-
}
1906-
1907-
return TPromise.as(undefined);
1908-
});
1904+
this.notificationService.prompt(
1905+
Severity.Info,
1906+
localize('InstallVSIXAction.success', "Successfully installed the extension. Reload to enable it."),
1907+
[{
1908+
label: localize('InstallVSIXAction.reloadNow', "Reload Now"),
1909+
run: () => this.windowService.reloadWindow()
1910+
}]
1911+
);
19091912
});
19101913
});
19111914
}
19121915
}
19131916

1914-
19151917
export class ReinstallAction extends Action {
19161918

19171919
static readonly ID = 'workbench.extensions.action.reinstall';
@@ -1955,11 +1957,14 @@ export class ReinstallAction extends Action {
19551957
private reinstallExtension(extension: IExtension): TPromise<void> {
19561958
return this.extensionsWorkbenchService.reinstall(extension)
19571959
.then(() => {
1958-
this.notificationService.prompt(Severity.Info, localize('ReinstallAction.success', "Successfully reinstalled the extension."), [localize('ReinstallAction.reloadNow', "Reload Now")]).done(choice => {
1959-
if (choice === 0) {
1960-
this.windowService.reloadWindow();
1961-
}
1962-
});
1960+
this.notificationService.prompt(
1961+
Severity.Info,
1962+
localize('ReinstallAction.success', "Successfully reinstalled the extension."),
1963+
[{
1964+
label: localize('ReinstallAction.reloadNow', "Reload Now"),
1965+
run: () => this.windowService.reloadWindow()
1966+
}]
1967+
);
19631968
}, error => this.notificationService.error(error));
19641969
}
19651970
}

0 commit comments

Comments
 (0)