Simplify button logic and enable button updates for custom dialogs (#1283)

src/sql/platform/dialog/customDialogService.ts

@@ -15,11 +15,21 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
 const defaultOptions: IModalOptions = { hasBackButton: true, isWide: false };
 
 export class CustomDialogService {
+	private _dialogModals = new Map<Dialog, DialogModal>();
+
 	constructor( @IInstantiationService private _instantiationService: IInstantiationService) { }
 
 	public showDialog(dialog: Dialog, options?: IModalOptions): void {
-		let optionsDialog = this._instantiationService.createInstance(DialogModal, dialog, 'CustomDialog', options || defaultOptions);
+		let dialogModal = this._instantiationService.createInstance(DialogModal, dialog, 'CustomDialog', options || defaultOptions);
-		optionsDialog.render();
+		this._dialogModals.set(dialog, dialogModal);
-		optionsDialog.open();
+		dialogModal.render();
+		dialogModal.open();
+	}
+
+	public closeDialog(dialog: Dialog): void {
+		let dialogModal = this._dialogModals.get(dialog);
+		if (dialogModal) {
+			dialogModal.cancel();
+		}
 	}
 }

src/sql/platform/dialog/dialogModal.ts

@@ -61,25 +61,33 @@ export class DialogModal extends Modal {
 		if (this._dialog.customButtons) {
 			this._dialog.customButtons.forEach(button => {
 				let buttonElement = this.addDialogButton(button);
-				buttonElement.enabled = button.enabled;
+				this.updateButtonElement(buttonElement, button);
-				attachButtonStyler(buttonElement, this._themeService);
 			});
 		}
 
 		this._cancelButton = this.addDialogButton(this._dialog.cancelButton, () => this.cancel());
-		this._cancelButton.enabled = this._dialog.cancelButton.enabled;
+		this.updateButtonElement(this._cancelButton, this._dialog.cancelButton);
 		this._doneButton = this.addDialogButton(this._dialog.okButton, () => this.done());
-		this._doneButton.enabled = this._dialog.okButton.enabled;
+		this.updateButtonElement(this._doneButton, this._dialog.okButton);
-		attachButtonStyler(this._cancelButton, this._themeService);
-		attachButtonStyler(this._doneButton, this._themeService);
 	}
 
 	private addDialogButton(button: DialogButton, onSelect: () => void = () => undefined): Button {
 		let buttonElement = this.addFooterButton(button.label, onSelect);
+		buttonElement.enabled = button.enabled;
 		button.registerClickEvent(buttonElement.onDidClick);
+		button.onUpdate(() => {
+			this.updateButtonElement(buttonElement, button);
+		});
+		attachButtonStyler(buttonElement, this._themeService);
 		return buttonElement;
 	}
 
+	private updateButtonElement(buttonElement: Button, dialogButton: DialogButton) {
+		buttonElement.label = dialogButton.label;
+		buttonElement.enabled = dialogButton.enabled;
+		dialogButton.hidden ? buttonElement.element.classList.add('dialogModal-hidden') : buttonElement.element.classList.remove('dialogModal-hidden');
+	}
+
 	protected renderBody(container: HTMLElement): void {
 		new Builder(container).div({ class: 'dialogModal-body' }, (bodyBuilder) => {
 			this._body = bodyBuilder.getHTMLElement();

src/sql/platform/dialog/dialogPane.ts

@@ -92,4 +92,9 @@ export class DialogPane extends Disposable implements IThemable {
 		this._body.style.backgroundColor = styles.dialogBodyBackground ? styles.dialogBodyBackground.toString() : undefined;
 		this._body.style.color = styles.dialogForeground ? styles.dialogForeground.toString() : undefined;
 	}
+
+	public dispose() {
+		super.dispose();
+		this._moduleRef.destroy();
+	}
 }

src/sql/platform/dialog/dialogTypes.ts

@@ -17,8 +17,6 @@ export class DialogTab implements sqlops.window.modelviewdialog.DialogTab {
 			this.content = content;
 		}
 	}
-
-	public updateContent(): void { }
 }
 
 export class Dialog implements sqlops.window.modelviewdialog.Dialog {
@@ -35,21 +33,48 @@ export class Dialog implements sqlops.window.modelviewdialog.Dialog {
 			this.content = content;
 		}
 	}
-
-	public open(): void { }
-	public close(): void { }
-	public updateContent(): void { }
 }
 
 export class DialogButton implements sqlops.window.modelviewdialog.Button {
-	public label: string;
+	private _label: string;
-	public enabled: boolean;
+	private _enabled: boolean;
+	private _hidden: boolean;
 	private _onClick: Emitter<void> = new Emitter<void>();
 	public readonly onClick: Event<void> = this._onClick.event;
+	private _onUpdate: Emitter<void> = new Emitter<void>();
+	public readonly onUpdate: Event<void> = this._onUpdate.event;
 
 	constructor(label: string, enabled: boolean) {
-		this.label = label;
+		this._label = label;
-		this.enabled = enabled;
+		this._enabled = enabled;
+		this._hidden = false;
+	}
+
+	public get label(): string {
+		return this._label;
+	}
+
+	public set label(label: string) {
+		this._label = label;
+		this._onUpdate.fire();
+	}
+
+	public get enabled(): boolean {
+		return this._enabled;
+	}
+
+	public set enabled(enabled: boolean) {
+		this._enabled = enabled;
+		this._onUpdate.fire();
+	}
+
+	public get hidden(): boolean {
+		return this._hidden;
+	}
+
+	public set hidden(hidden: boolean) {
+		this._hidden = hidden;
+		this._onUpdate.fire();
 	}
 
 	/**

src/sql/platform/dialog/media/dialogModal.css

@@ -23,6 +23,6 @@
 	height: 100%;
 }
 
-.dialogModal-pane.dialogModal-hidden {
+.dialogModal-hidden {
 	display: none;
 }

src/sql/sqlops.proposed.d.ts

@@ -287,6 +287,16 @@ declare module 'sqlops' {
 			 */
 			export function createButton(label: string): Button;
 
+			/**
+			 * Opens the given dialog if it is not already open
+			 */
+			export function openDialog(dialog: Dialog): void;
+
+			/**
+			 * Closes the given dialog if it is open
+			 */
+			export function closeDialog(dialog: Dialog): void;
+
 			// Model view dialog classes
 			export interface Dialog {
 				/**
@@ -297,7 +307,6 @@ declare module 'sqlops' {
 				/**
 				 * The content of the dialog. If multiple tabs are given they will be displayed with tabs
 				 * If a string is given, it should be the ID of the dialog's model view content
-				 * TODO mairvine 4/18/18: use a model view content type
 				 */
 				content: string | DialogTab[],
 
@@ -315,51 +324,35 @@ declare module 'sqlops' {
 				 * Any additional buttons that should be displayed
 				 */
 				customButtons: Button[];
-
-				/**
-				 * Opens the dialog
-				 */
-				open(): void;
-
-				/**
-				 * Closes the dialog
-				 */
-				close(): void;
-
-				/**
-				 * Updates the dialog on screen to reflect changes to the buttons or content
-				 */
-				updateContent(): void;
 			}
 
 			export interface DialogTab {
 				/**
 				 * The title of the tab
 				 */
-				title: string,
+				title: string;
 
 				/**
 				 * A string giving the ID of the tab's model view content
-				 * TODO mairvine 4/18/18: use a model view content type
 				 */
 				content: string;
-
-				/**
-				 * Updates the dialog on screen to reflect changes to the content
-				 */
-				updateContent(): void;
 			}
 
 			export interface Button {
 				/**
 				 * The label displayed on the button
 				 */
-				label: string,
+				label: string;
 
 				/**
 				 * Whether the button is enabled
 				 */
-				enabled: boolean,
+				enabled: boolean;
+
+				/**
+				 * Whether the button is hidden
+				 */
+				hidden: boolean;
 
 				/**
 				 * Raised when the button is clicked

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

@@ -115,4 +115,5 @@ export interface IModelViewTabDetails {
 export interface IModelViewButtonDetails {
 	label: string;
 	enabled: boolean;
+	hidden: boolean;
 }

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

@@ -26,18 +26,6 @@ class DialogImpl implements sqlops.window.modelviewdialog.Dialog {
 		this.okButton = this._extHostModelViewDialog.createButton(nls.localize('dialogOkLabel', 'Done'));
 		this.cancelButton = this._extHostModelViewDialog.createButton(nls.localize('dialogCancelLabel', 'Cancel'));
 	}
-
-	public open(): void {
-		this._extHostModelViewDialog.open(this);
-	}
-
-	public close(): void {
-		this._extHostModelViewDialog.close(this);
-	}
-
-	public updateContent(): void {
-		this._extHostModelViewDialog.updateDialogContent(this);
-	}
 }
 
 class TabImpl implements sqlops.window.modelviewdialog.DialogTab {
@@ -45,21 +33,19 @@ class TabImpl implements sqlops.window.modelviewdialog.DialogTab {
 	public content: string;
 
 	constructor(private _extHostModelViewDialog: ExtHostModelViewDialog) { }
-
-	public updateContent(): void {
-		this._extHostModelViewDialog.updateTabContent(this);
-	}
 }
 
 class ButtonImpl implements sqlops.window.modelviewdialog.Button {
 	private _label: string;
 	private _enabled: boolean;
+	private _hidden: boolean;
 
 	private _onClick = new Emitter<void>();
 	public onClick = this._onClick.event;
 
 	constructor(private _extHostModelViewDialog: ExtHostModelViewDialog) {
 		this._enabled = true;
+		this._hidden = false;
 	}
 
 	public get label(): string {
@@ -80,6 +66,15 @@ class ButtonImpl implements sqlops.window.modelviewdialog.Button {
 		this._extHostModelViewDialog.updateButton(this);
 	}
 
+	public get hidden(): boolean {
+		return this._hidden;
+	}
+
+	public set hidden(hidden: boolean) {
+		this._hidden = hidden;
+		this._extHostModelViewDialog.updateButton(this);
+	}
+
 	public getOnClickCallback(): () => void {
 		return () => this._onClick.fire();
 	}
@@ -154,7 +149,7 @@ export class ExtHostModelViewDialog implements ExtHostModelViewDialogShape {
 		let handle = this.getDialogHandle(dialog);
 		let tabs = dialog.content;
 		if (tabs && typeof tabs !== 'string') {
-			tabs.forEach(tab => tab.updateContent());
+			tabs.forEach(tab => this.updateTabContent(tab));
 		}
 		if (dialog.customButtons) {
 			dialog.customButtons.forEach(button => this.updateButton(button));
@@ -182,7 +177,8 @@ export class ExtHostModelViewDialog implements ExtHostModelViewDialogShape {
 		let handle = this.getButtonHandle(button);
 		this._proxy.$setButtonDetails(handle, {
 			label: button.label,
-			enabled: button.enabled
+			enabled: button.enabled,
+			hidden: button.hidden
 		});
 	}
 

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

@@ -22,7 +22,7 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape
 
 	constructor(
 		context: IExtHostContext,
-		@IInstantiationService instatiationService: IInstantiationService
+		@IInstantiationService instatiationService: IInstantiationService,
 	) {
 		this._proxy = context.getProxy(SqlExtHostContext.ExtHostModelViewDialog);
 		this._dialogService = new CustomDialogService(instatiationService);
@@ -40,7 +40,7 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape
 
 	public $close(handle: number): Thenable<void> {
 		let dialog = this.getDialog(handle);
-		dialog.close();
+		this._dialogService.closeDialog(dialog);
 		return Promise.resolve();
 	}
 
@@ -66,7 +66,6 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape
 			dialog.customButtons = details.customButtons.map(buttonHandle => this.getButton(buttonHandle));
 		}
 
-		dialog.updateContent();
 		return Promise.resolve();
 	}
 
@@ -79,8 +78,6 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape
 
 		tab.title = details.title;
 		tab.content = details.content;
-
-		tab.updateContent();
 		return Promise.resolve();
 	}
 
@@ -88,11 +85,13 @@ export class MainThreadModelViewDialog implements MainThreadModelViewDialogShape
 		let button = this._buttons.get(handle);
 		if (!button) {
 			button = new DialogButton(details.label, details.enabled);
+			button.hidden = details.hidden;
 			button.onClick(() => this.onButtonClick(handle));
 			this._buttons.set(handle, button);
 		} else {
 			button.label = details.label;
 			button.enabled = details.enabled;
+			button.hidden = details.hidden;
 		}
 
 		return Promise.resolve();

src/sql/workbench/api/node/sqlExtHost.api.impl.ts

@@ -293,6 +293,12 @@ export function createApiFactory(
 				},
 				createButton(label: string): sqlops.window.modelviewdialog.Button {
 					return extHostModelViewDialog.createButton(label);
+				},
+				openDialog(dialog: sqlops.window.modelviewdialog.Dialog) {
+					return extHostModelViewDialog.open(dialog);
+				},
+				closeDialog(dialog: sqlops.window.modelviewdialog.Dialog) {
+					return extHostModelViewDialog.close(dialog);
 				}
 			};
 

src/sqltest/workbench/api/mainThreadModelViewDialog.test.ts

@@ -55,19 +55,23 @@ suite('MainThreadModelViewDialog Tests', () => {
 		// Set up the dialog details
 		button1Details = {
 			label: 'button1',
-			enabled: false
+			enabled: false,
+			hidden: false
 		};
 		button2Details = {
 			label: 'button2',
-			enabled: true
+			enabled: true,
+			hidden: false
 		};
 		okButtonDetails = {
 			label: 'ok_label',
-			enabled: true
+			enabled: true,
+			hidden: false
 		};
 		cancelButtonDetails = {
 			label: 'cancel_label',
-			enabled: true
+			enabled: true,
+			hidden: false
 		};
 		tab1Details = {
 			title: 'tab1',