Add onClosed event to ModelView dialogs (#17531)

* Add onClosed event to ModelView dialogs

* Use defined type

* Fix compile

* fix tests

* fix tests2

* Remove unused

extensions/machine-learning/src/test/views/utils.ts

@@ -303,6 +303,7 @@ export function createViewContext(): ViewTestContext {
 		registerCloseValidator: () => { },
 		registerOperation: () => { },
 		onValidityChanged: new vscode.EventEmitter<boolean>().event,
+		onClosed: new vscode.EventEmitter<azdata.window.CloseReason>().event,
 		registerContent: () => { },
 		modelView: undefined!,
 		valid: true

extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts

@@ -55,6 +55,7 @@ export class PublishDatabaseDialog {
 
 	constructor(private project: Project) {
 		this.dialog = utils.getAzdataApi()!.window.createModelViewDialog(constants.publishDialogName, 'sqlProjectPublishDialog');
+		this.toDispose.push(this.dialog.onClosed(_ => this.completionPromise.resolve()));
 		this.publishTab = utils.getAzdataApi()!.window.createTab(constants.publishDialogName);
 	}
 
@@ -74,9 +75,6 @@ export class PublishDatabaseDialog {
 		this.dialog.customButtons.push(generateScriptButton);
 
 		utils.getAzdataApi()!.window.openDialog(this.dialog);
-		// Complete the promise when we're done - we use a disposable here instead of a CloseValidator because we have custom buttons (generate script)
-		// which don't go through that logic. 
-		this.toDispose.push({ dispose: () => { this.completionPromise.resolve(); } });
 	}
 
 	public waitForClose(): Promise<void> {

extensions/sql-database-projects/src/test/projectController.test.ts

@@ -395,15 +395,13 @@ describe('ProjectsController', function (): void {
 					return Promise.resolve(undefined);
 				});
 
-				let dialogPromise = projController.object.publishProject(proj);
+				void projController.object.publishProject(proj);
 				await publishDialog.object.publishClick();
-				await dialogPromise;
 
 				should(holler).equal(publishHoller, 'executionCallback() is supposed to have been setup and called for Publish scenario');
 
-				dialogPromise = projController.object.publishProject(proj);
+				void projController.object.publishProject(proj);
 				await publishDialog.object.generateScriptClick();
-				await dialogPromise;
 
 				should(holler).equal(generateHoller, 'executionCallback() is supposed to have been setup and called for GenerateScript scenario');
 			});

src/sql/azdata.proposed.d.ts

@@ -579,6 +579,11 @@ declare module 'azdata' {
 
 	export namespace window {
 
+		/**
+		 * The reason that the dialog was closed
+		 */
+		export type CloseReason = 'close' | 'cancel' | 'ok';
+
 		export interface Dialog {
 			/**
 			 * Width of the dialog.
@@ -610,6 +615,11 @@ declare module 'azdata' {
 			 * Default is undefined.
 			 */
 			dialogProperties?: IDialogProperties;
+
+			/**
+			 * Fired when the dialog is closed for any reason. The value indicates the reason it was closed (such as 'ok' or 'cancel')
+			 */
+			onClosed: vscode.Event<CloseReason>;
 		}
 
 		export interface Wizard {

src/sql/workbench/api/browser/mainThreadModelViewDialog.ts

@@ -98,7 +98,11 @@ export class MainThreadModelViewDialog extends Disposable implements MainThreadM
 		options.renderHeader = dialog.renderHeader;
 		options.renderFooter = dialog.renderFooter;
 		options.dialogProperties = dialog.dialogProperties;
-		this._dialogService.showDialog(dialog, dialogName, options);
+		const modal = this._dialogService.showDialog(dialog, dialogName, options);
+		const onClosed = modal.onClosed(reason => {
+			this._proxy.$onClosed(handle, reason);
+			onClosed.dispose();
+		});
 		return Promise.resolve();
 	}
 

src/sql/workbench/api/common/extHostModelViewDialog.ts

@@ -133,6 +133,9 @@ class DialogImpl extends ModelViewPanelImpl implements azdata.window.Dialog {
 	private _renderFooter: boolean;
 	private _dialogProperties: IDialogProperties;
 
+	private _onClosed = new Emitter<azdata.window.CloseReason>();
+	public onClosed = this._onClosed.event;
+
 	constructor(extHostModelViewDialog: ExtHostModelViewDialog,
 		extHostModelView: ExtHostModelViewShape,
 		extHostTaskManagement: ExtHostBackgroundTaskManagementShape,
@@ -239,6 +242,10 @@ class DialogImpl extends ModelViewPanelImpl implements azdata.window.Dialog {
 			return Promise.resolve(true);
 		}
 	}
+
+	public handleOnClosed(reason: azdata.window.CloseReason): void {
+		this._onClosed.fire(reason);
+	}
 }
 
 class TabImpl extends ModelViewPanelImpl implements azdata.window.DialogTab {
@@ -697,6 +704,11 @@ export class ExtHostModelViewDialog implements ExtHostModelViewDialogShape {
 		return editor.handleSave();
 	}
 
+	public $onClosed(handle: number, reason: azdata.window.CloseReason): void {
+		let dialog = this._objectsByHandle.get(handle) as DialogImpl;
+		dialog.handleOnClosed(reason);
+	}
+
 	public openDialog(dialog: azdata.window.Dialog): void {
 		let handle = this.getHandle(dialog);
 		this.updateDialogContent(dialog);

src/sql/workbench/api/common/sqlExtHost.protocol.ts

@@ -851,6 +851,7 @@ export interface ExtHostModelViewDialogShape {
 	$validateNavigation(handle: number, info: azdata.window.WizardPageChangeInfo): Thenable<boolean>;
 	$validateDialogClose(handle: number): Thenable<boolean>;
 	$handleSave(handle: number): Thenable<boolean>;
+	$onClosed(handle: number, reason: azdata.window.CloseReason): void;
 }
 
 export interface MainThreadModelViewDialogShape extends IDisposable {

src/sql/workbench/browser/modal/modal.ts

@@ -29,6 +29,7 @@ import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
 import { ILogService } from 'vs/platform/log/common/log';
 import { attachButtonStyler } from 'vs/platform/theme/common/styler';
 import { IThemeService } from 'vs/platform/theme/common/themeService';
+import { Emitter } from 'vs/base/common/event';
 
 export enum MessageLevel {
 	Error = 0,
@@ -147,6 +148,9 @@ export abstract class Modal extends Disposable implements IThemable {
 	private _modalShowingContext: IContextKey<Array<string>>;
 	private readonly _staticKey: string;
 
+	private _onClosed = new Emitter<HideReason>();
+	public onClosed = this._onClosed.event;
+
 	/**
 	 * Get the back button, only available after render and if the hasBackButton option is true
 	 */
@@ -458,7 +462,7 @@ export abstract class Modal extends Disposable implements IThemable {
 			}
 		}));
 		this.disposableStore.add(trapKeyboardNavigation(this._modalDialog!));
-		this.disposableStore.add(DOM.addDisposableListener(window, DOM.EventType.RESIZE, (e: Event) => {
+		this.disposableStore.add(DOM.addDisposableListener(window, DOM.EventType.RESIZE, e => {
 			this.layout(DOM.getTotalHeight(this._modalBodySection!));
 		}));
 
@@ -476,7 +480,7 @@ export abstract class Modal extends Disposable implements IThemable {
 	/**
 	 * Hides the modal and removes key listeners
 	 */
-	protected hide(reason?: HideReason, currentPageName?: string): void {
+	protected hide(reason: HideReason = 'close', currentPageName?: string): void {
 		this._modalShowingContext.get()!.pop();
 		this._bodyContainer!.remove();
 		this.disposableStore.clear();
@@ -488,6 +492,7 @@ export abstract class Modal extends Disposable implements IThemable {
 			})
 			.send();
 		this.restoreKeyboardFocus();
+		this._onClosed.fire(reason);
 	}
 
 	private restoreKeyboardFocus() {

src/sql/workbench/contrib/webview/browser/webViewDialog.ts

@@ -33,8 +33,6 @@ export class WebViewDialog extends Modal {
 
 	private _onOk = new Emitter<void>();
 	public onOk: Event<void> = this._onOk.event;
-	private _onClosed = new Emitter<void>();
-	public onClosed: Event<void> = this._onClosed.event;
 	private _onMessage = new Emitter<any>();
 
 	private readonly id = generateUuid();
@@ -141,7 +139,6 @@ export class WebViewDialog extends Modal {
 
 	public close(hideReason: HideReason = 'close') {
 		this.hide(hideReason);
-		this._onClosed.fire();
 	}
 
 	public sendMessage(message: any): void {

src/sql/workbench/services/dialog/browser/customDialogService.ts

@@ -18,7 +18,7 @@ export class CustomDialogService {
 
 	constructor(@IInstantiationService private _instantiationService: IInstantiationService) { }
 
-	public showDialog(dialog: Dialog, dialogName?: string, options?: IModalOptions): void {
+	public showDialog(dialog: Dialog, dialogName?: string, options?: IModalOptions): DialogModal {
 		let name = dialogName ? dialogName : 'CustomDialog';
 
 		if (options && (options.dialogStyle === 'callout')) {
@@ -30,6 +30,7 @@ export class CustomDialogService {
 		this._dialogModals.set(dialog, dialogModal);
 		dialogModal.render();
 		dialogModal.open();
+		return dialogModal;
 	}
 
 	public showWizard(wizard: Wizard, options?: IModalOptions, source?: string): void {

src/sql/workbench/services/dialog/test/browser/testDialogModal.ts

@@ -0,0 +1,31 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { IWorkbenchThemeService } from 'vs/workbench/services/themes/common/workbenchThemeService';
+import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
+import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
+import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
+import { ILogService } from 'vs/platform/log/common/log';
+import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService';
+import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry';
+import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
+import { DialogModal } from 'sql/workbench/services/dialog/browser/dialogModal';
+import { Dialog } from 'sql/workbench/services/dialog/common/dialogTypes';
+
+export class TestDialogModal extends DialogModal {
+
+	constructor(
+		@ILayoutService layoutService: ILayoutService,
+		@IWorkbenchThemeService themeService: IWorkbenchThemeService,
+		@IAdsTelemetryService telemetryService: IAdsTelemetryService,
+		@IContextKeyService contextKeyService: IContextKeyService,
+		@IClipboardService clipboardService: IClipboardService,
+		@ILogService logService: ILogService,
+		@ITextResourcePropertiesService textResourcePropertiesService: ITextResourcePropertiesService,
+		@IInstantiationService instantiationService: IInstantiationService) {
+		// For now just hardcode the dialog since current uses just care about the title property
+		super(<Dialog>{ title: 'TestDialogModal' }, '', {}, layoutService, themeService, telemetryService, contextKeyService, clipboardService, logService, textResourcePropertiesService, instantiationService);
+	}
+}

src/sql/workbench/test/electron-browser/api/mainThreadModelViewDialog.test.ts

@@ -12,6 +12,10 @@ import { CustomDialogService } from 'sql/workbench/services/dialog/browser/custo
 import { Dialog, DialogTab, Wizard } from 'sql/workbench/services/dialog/common/dialogTypes';
 import { ExtHostModelViewDialogShape } from 'sql/workbench/api/common/sqlExtHost.protocol';
 import { Emitter } from 'vs/base/common/event';
+import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
+import { TestDialogModal } from 'sql/workbench/services/dialog/test/browser/testDialogModal';
+import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
+import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 
 
 suite('MainThreadModelViewDialog Tests', () => {
@@ -70,10 +74,15 @@ suite('MainThreadModelViewDialog Tests', () => {
 
 		// Set up the mock dialog service
 		mockDialogService = Mock.ofType(CustomDialogService, undefined);
+		const testInstantiationService = new TestInstantiationService();
+		testInstantiationService.set(IContextKeyService, new MockContextKeyService());
+		const testDialogModal = testInstantiationService.createInstance(TestDialogModal);
 		openedDialog = undefined;
 		mockDialogService.setup(x => x.showDialog(It.isAny(), undefined, It.isAny())).callback((dialog) => {
 			openedDialog = dialog;
-		});
+			return {};
+		}).returns(() => testDialogModal);
+
 		mockDialogService.setup(x => x.showWizard(It.isAny(), It.isAny(), It.isAny())).callback(wizard => {
 			openedWizard = wizard;
 			// The actual service will set the page to 0 when it opens the wizard