diff --git a/extensions/machine-learning/src/test/views/utils.ts b/extensions/machine-learning/src/test/views/utils.ts index dd23bdf796..8e07391ab3 100644 --- a/extensions/machine-learning/src/test/views/utils.ts +++ b/extensions/machine-learning/src/test/views/utils.ts @@ -303,6 +303,7 @@ export function createViewContext(): ViewTestContext { registerCloseValidator: () => { }, registerOperation: () => { }, onValidityChanged: new vscode.EventEmitter().event, + onClosed: new vscode.EventEmitter().event, registerContent: () => { }, modelView: undefined!, valid: true diff --git a/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts b/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts index 322c982ca8..d565757378 100644 --- a/extensions/sql-database-projects/src/dialogs/publishDatabaseDialog.ts +++ b/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 { diff --git a/extensions/sql-database-projects/src/test/projectController.test.ts b/extensions/sql-database-projects/src/test/projectController.test.ts index d9ffd973f4..a9e65aa3a9 100644 --- a/extensions/sql-database-projects/src/test/projectController.test.ts +++ b/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'); }); diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index ed39baa0d0..718c69c2d1 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/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; } export interface Wizard { diff --git a/src/sql/workbench/api/browser/mainThreadModelViewDialog.ts b/src/sql/workbench/api/browser/mainThreadModelViewDialog.ts index 2ca833c639..8a49375fc6 100644 --- a/src/sql/workbench/api/browser/mainThreadModelViewDialog.ts +++ b/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(); } diff --git a/src/sql/workbench/api/common/extHostModelViewDialog.ts b/src/sql/workbench/api/common/extHostModelViewDialog.ts index c70dbd6a98..e01ce2c0c4 100644 --- a/src/sql/workbench/api/common/extHostModelViewDialog.ts +++ b/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(); + 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); diff --git a/src/sql/workbench/api/common/sqlExtHost.protocol.ts b/src/sql/workbench/api/common/sqlExtHost.protocol.ts index e56840fe71..04728c4320 100644 --- a/src/sql/workbench/api/common/sqlExtHost.protocol.ts +++ b/src/sql/workbench/api/common/sqlExtHost.protocol.ts @@ -851,6 +851,7 @@ export interface ExtHostModelViewDialogShape { $validateNavigation(handle: number, info: azdata.window.WizardPageChangeInfo): Thenable; $validateDialogClose(handle: number): Thenable; $handleSave(handle: number): Thenable; + $onClosed(handle: number, reason: azdata.window.CloseReason): void; } export interface MainThreadModelViewDialogShape extends IDisposable { diff --git a/src/sql/workbench/browser/modal/modal.ts b/src/sql/workbench/browser/modal/modal.ts index c33e0a612d..061c22c2ae 100644 --- a/src/sql/workbench/browser/modal/modal.ts +++ b/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>; private readonly _staticKey: string; + private _onClosed = new Emitter(); + 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() { diff --git a/src/sql/workbench/contrib/webview/browser/webViewDialog.ts b/src/sql/workbench/contrib/webview/browser/webViewDialog.ts index 11ddb79b98..7c9b248555 100644 --- a/src/sql/workbench/contrib/webview/browser/webViewDialog.ts +++ b/src/sql/workbench/contrib/webview/browser/webViewDialog.ts @@ -33,8 +33,6 @@ export class WebViewDialog extends Modal { private _onOk = new Emitter(); public onOk: Event = this._onOk.event; - private _onClosed = new Emitter(); - public onClosed: Event = this._onClosed.event; private _onMessage = new Emitter(); 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 { diff --git a/src/sql/workbench/services/dialog/browser/customDialogService.ts b/src/sql/workbench/services/dialog/browser/customDialogService.ts index 817462e0ad..79b8e292ac 100644 --- a/src/sql/workbench/services/dialog/browser/customDialogService.ts +++ b/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 { diff --git a/src/sql/workbench/services/dialog/test/browser/testDialogModal.ts b/src/sql/workbench/services/dialog/test/browser/testDialogModal.ts new file mode 100644 index 0000000000..e13882b5cc --- /dev/null +++ b/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({ title: 'TestDialogModal' }, '', {}, layoutService, themeService, telemetryService, contextKeyService, clipboardService, logService, textResourcePropertiesService, instantiationService); + } +} diff --git a/src/sql/workbench/test/electron-browser/api/mainThreadModelViewDialog.test.ts b/src/sql/workbench/test/electron-browser/api/mainThreadModelViewDialog.test.ts index 9fd0ea02e9..6de2fdb857 100644 --- a/src/sql/workbench/test/electron-browser/api/mainThreadModelViewDialog.test.ts +++ b/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