From ea1780364b9ff8aeadadbddb4e46d9f0a3fab74d Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 29 Jun 2016 20:09:58 -0700 Subject: [PATCH] [Dialog Service] Dismiss individual dialogs. Fixes #254 --- .../src/DialogLaunchController.js | 126 ++++++++++-------- platform/commonUI/dialog/src/DialogService.js | 67 +++++----- .../commonUI/dialog/test/DialogServiceSpec.js | 41 +++++- .../edit/src/actions/SaveInProgressDialog.js | 7 +- .../edit/test/actions/SaveActionSpec.js | 33 +++-- .../edit/test/actions/SaveAsActionSpec.js | 33 +++-- .../src/controllers/BannerController.js | 8 +- .../src/NotificationIndicatorController.js | 3 - .../NotificationIndicatorControllerSpec.js | 15 --- .../entanglement/src/actions/CopyAction.js | 13 +- .../test/actions/CopyActionSpec.js | 6 +- 11 files changed, 209 insertions(+), 143 deletions(-) diff --git a/example/notifications/src/DialogLaunchController.js b/example/notifications/src/DialogLaunchController.js index f35d008cd0..e7f867d303 100644 --- a/example/notifications/src/DialogLaunchController.js +++ b/example/notifications/src/DialogLaunchController.js @@ -44,30 +44,31 @@ define( periodically with the progress of an ongoing process. */ $scope.launchProgress = function (knownProgress) { - var model = { - title: "Progress Dialog Example", - progress: 0, - hint: "Do not navigate away from this page or close this browser tab while this operation is in progress.", - actionText: "Calculating...", - unknownProgress: !knownProgress, - unknownDuration: false, - severity: "info", - options: [ - { - label: "Cancel Operation", - callback: function () { - $log.debug("Operation cancelled"); - dialogService.dismiss(); + var dialog, + model = { + title: "Progress Dialog Example", + progress: 0, + hint: "Do not navigate away from this page or close this browser tab while this operation is in progress.", + actionText: "Calculating...", + unknownProgress: !knownProgress, + unknownDuration: false, + severity: "info", + options: [ + { + label: "Cancel Operation", + callback: function () { + $log.debug("Operation cancelled"); + dialog.dismiss(); + } + }, + { + label: "Do something else...", + callback: function () { + $log.debug("Something else pressed"); + } } - }, - { - label: "Do something else...", - callback: function () { - $log.debug("Something else pressed"); - } - } - ] - }; + ] + }; function incrementProgress() { model.progress = Math.min(100, Math.floor(model.progress + Math.random() * 30)); @@ -77,7 +78,9 @@ define( } } - if (dialogService.showBlockingMessage(model)) { + dialog = dialogService.showBlockingMessage(model); + + if (dialog) { //Do processing here model.actionText = "Processing 100 objects..."; if (knownProgress) { @@ -93,29 +96,31 @@ define( Demonstrates launching an error dialog */ $scope.launchError = function () { - var model = { - title: "Error Dialog Example", - actionText: "Something happened, and it was not good.", - severity: "error", - options: [ - { - label: "Try Again", - callback: function () { - $log.debug("Try Again Pressed"); - dialogService.dismiss(); + var dialog, + model = { + title: "Error Dialog Example", + actionText: "Something happened, and it was not good.", + severity: "error", + options: [ + { + label: "Try Again", + callback: function () { + $log.debug("Try Again Pressed"); + dialog.dismiss(); + } + }, + { + label: "Cancel", + callback: function () { + $log.debug("Cancel Pressed"); + dialog.dismiss(); + } } - }, - { - label: "Cancel", - callback: function () { - $log.debug("Cancel Pressed"); - dialogService.dismiss(); - } - } - ] - }; + ] + }; + dialog = dialogService.showBlockingMessage(model); - if (!dialogService.showBlockingMessage(model)) { + if (!dialog) { $log.error("Could not display modal dialog"); } }; @@ -124,22 +129,25 @@ define( Demonstrates launching an error dialog */ $scope.launchInfo = function () { - var model = { - title: "Info Dialog Example", - actionText: "This is an example of a blocking info" + - " dialog. This dialog can be used to draw the user's" + - " attention to an event.", - severity: "info", - primaryOption: { - label: "OK", - callback: function () { - $log.debug("OK Pressed"); - dialogService.dismiss(); + var dialog, + model = { + title: "Info Dialog Example", + actionText: "This is an example of a blocking info" + + " dialog. This dialog can be used to draw the user's" + + " attention to an event.", + severity: "info", + primaryOption: { + label: "OK", + callback: function () { + $log.debug("OK Pressed"); + dialog.dismiss(); + } } - } - }; + }; - if (!dialogService.showBlockingMessage(model)) { + dialog = dialogService.showBlockingMessage(model); + + if (!dialog) { $log.error("Could not display modal dialog"); } }; diff --git a/platform/commonUI/dialog/src/DialogService.js b/platform/commonUI/dialog/src/DialogService.js index 778a172147..f3e888b6e0 100644 --- a/platform/commonUI/dialog/src/DialogService.js +++ b/platform/commonUI/dialog/src/DialogService.js @@ -39,25 +39,28 @@ define( this.overlayService = overlayService; this.$q = $q; this.$log = $log; - this.overlay = undefined; - this.dialogVisible = false; + this.activeOverlay = undefined; } - // Stop showing whatever overlay is currently active - // (e.g. because the user hit cancel) - DialogService.prototype.dismiss = function () { - var overlay = this.overlay; - if (overlay) { - overlay.dismiss(); + /** + * @private + */ + DialogService.prototype.dismissOverlay = function (overlay) { + //Dismiss the overlay + overlay.dismiss(); + + //If dialog is the current active one, dismiss it + if (overlay === this.activeOverlay) { + this.activeOverlay = undefined; } - this.dialogVisible = false; }; DialogService.prototype.getDialogResponse = function (key, model, resultGetter, typeClass) { // We will return this result as a promise, because user // input is asynchronous. var deferred = this.$q.defer(), - self = this; + self = this, + overlay; // Confirm function; this will be passed in to the // overlay-dialog template and associated with a @@ -65,9 +68,7 @@ define( function confirm(value) { // Pass along the result deferred.resolve(resultGetter ? resultGetter() : value); - - // Stop showing the dialog - self.dismiss(); + self.dismissOverlay(overlay); } // Cancel function; this will be passed in to the @@ -75,7 +76,7 @@ define( // Cancel or X button click function cancel() { deferred.reject(); - self.dismiss(); + self.dismissOverlay(overlay); } // Add confirm/cancel callbacks @@ -85,15 +86,11 @@ define( if (this.canShowDialog(model)) { // Add the overlay using the OverlayService, which // will handle actual insertion into the DOM - this.overlay = this.overlayService.createOverlay( + overlay = this.activeOverlay = this.overlayService.createOverlay( key, model, typeClass || "t-dialog" ); - - // Track that a dialog is already visible, to - // avoid spawning multiple dialogs at once. - this.dialogVisible = true; } else { deferred.reject(); } @@ -156,7 +153,7 @@ define( * otherwise */ DialogService.prototype.canShowDialog = function (dialogModel) { - if (this.dialogVisible) { + if (this.activeOverlay) { // Only one dialog should be shown at a time. // The application design should be such that // we never even try to do this. @@ -183,6 +180,11 @@ define( * button is clicked */ + /** + * @typedef DialogHandle + * @property {function} dismiss a function to dismiss the given dialog + */ + /** * A description of the model options that may be passed to the * showBlockingMessage method. Note that the DialogModel desribed @@ -222,21 +224,26 @@ define( * the user can take if necessary * @param {DialogModel} dialogModel defines options for the dialog * @param {typeClass} string tells overlayService that this overlay should use appropriate CSS class - * @returns {boolean} + * @returns {boolean | {DialogHandle}} */ DialogService.prototype.showBlockingMessage = function (dialogModel) { if (this.canShowDialog(dialogModel)) { // Add the overlay using the OverlayService, which // will handle actual insertion into the DOM - this.overlay = this.overlayService.createOverlay( - "overlay-blocking-message", - dialogModel, - "t-dialog-sm" - ); - // Track that a dialog is already visible, to - // avoid spawning multiple dialogs at once. - this.dialogVisible = true; - return true; + var self = this, + overlay = this.overlayService.createOverlay( + "overlay-blocking-message", + dialogModel, + "t-dialog-sm" + ); + + this.activeOverlay = overlay; + + return { + dismiss: function () { + self.dismissOverlay(overlay); + } + }; } else { return false; } diff --git a/platform/commonUI/dialog/test/DialogServiceSpec.js b/platform/commonUI/dialog/test/DialogServiceSpec.js index 2d801eb028..663f9fbeda 100644 --- a/platform/commonUI/dialog/test/DialogServiceSpec.js +++ b/platform/commonUI/dialog/test/DialogServiceSpec.js @@ -122,7 +122,7 @@ define( it("invokes the overlay service with the correct parameters when" + " a blocking dialog is requested", function () { var dialogModel = {}; - expect(dialogService.showBlockingMessage(dialogModel)).toBe(true); + expect(dialogService.showBlockingMessage(dialogModel)).not.toBe(false); expect(mockOverlayService.createOverlay).toHaveBeenCalledWith( "overlay-blocking-message", dialogModel, @@ -130,6 +130,45 @@ define( ); }); + describe("the blocking message dialog", function () { + var dialogModel = {}; + var dialogHandle; + + beforeEach(function () { + dialogHandle = dialogService.showBlockingMessage(dialogModel); + }); + + it("returns a handle to the dialog", function () { + expect(dialogHandle).not.toBe(undefined); + }); + + it("dismissing the dialog dismisses the overlay", function () { + dialogHandle.dismiss(); + expect(mockOverlay.dismiss).toHaveBeenCalled(); + }); + + it("individual dialogs can be dismissed", function () { + var secondDialogHandle, + secondMockOverlay; + + dialogHandle.dismiss(); + + secondMockOverlay = jasmine.createSpyObj( + "overlay", + ["dismiss"] + ); + mockOverlayService.createOverlay.andReturn(secondMockOverlay); + secondDialogHandle = dialogService.showBlockingMessage(dialogModel); + + //Dismiss the first dialog. It should only dismiss if it + // is active + dialogHandle.dismiss(); + expect(secondMockOverlay.dismiss).not.toHaveBeenCalled(); + secondDialogHandle.dismiss(); + expect(secondMockOverlay.dismiss).toHaveBeenCalled(); + }); + }); + }); } ); diff --git a/platform/commonUI/edit/src/actions/SaveInProgressDialog.js b/platform/commonUI/edit/src/actions/SaveInProgressDialog.js index c80989b8e0..56c2c1ad86 100644 --- a/platform/commonUI/edit/src/actions/SaveInProgressDialog.js +++ b/platform/commonUI/edit/src/actions/SaveInProgressDialog.js @@ -1,10 +1,11 @@ define([], function () { function SaveInProgressDialog(dialogService) { this.dialogService = dialogService; + this.dialog = undefined; } SaveInProgressDialog.prototype.show = function () { - this.dialogService.showBlockingMessage({ + this.dialog = this.dialogService.showBlockingMessage({ title: "Saving...", hint: "Do not navigate away from this page or close this browser tab while this message is displayed.", unknownProgress: true, @@ -13,7 +14,9 @@ define([], function () { }; SaveInProgressDialog.prototype.hide = function () { - this.dialogService.dismiss(); + if (this.dialog) { + this.dialog.dismiss(); + } }; return SaveInProgressDialog; diff --git a/platform/commonUI/edit/test/actions/SaveActionSpec.js b/platform/commonUI/edit/test/actions/SaveActionSpec.js index b81da3d7de..a24e3f4587 100644 --- a/platform/commonUI/edit/test/actions/SaveActionSpec.js +++ b/platform/commonUI/edit/test/actions/SaveActionSpec.js @@ -70,7 +70,7 @@ define( }; dialogService = jasmine.createSpyObj( "dialogService", - ["showBlockingMessage", "dismiss"] + ["showBlockingMessage"] ); mockDomainObject.hasCapability.andReturn(true); @@ -111,17 +111,28 @@ define( expect(mockActionCapability.perform).toHaveBeenCalledWith("navigate"); }); - it("shows a dialog while saving", function () { - mockEditorCapability.save.andReturn(new Promise(function () {})); - action.perform(); - expect(dialogService.showBlockingMessage).toHaveBeenCalled(); - expect(dialogService.dismiss).not.toHaveBeenCalled(); - }); + describe("a blocking dialog", function () { + var mockDialogHandle; - it("hides a dialog when saving is complete", function () { - action.perform(); - expect(dialogService.showBlockingMessage).toHaveBeenCalled(); - expect(dialogService.dismiss).toHaveBeenCalled(); + beforeEach(function () { + mockDialogHandle = jasmine.createSpyObj("dialogHandle", ["dismiss"]); + dialogService.showBlockingMessage.andReturn(mockDialogHandle); + }); + + + it("shows a dialog while saving", function () { + mockEditorCapability.save.andReturn(new Promise(function () { + })); + action.perform(); + expect(dialogService.showBlockingMessage).toHaveBeenCalled(); + expect(mockDialogHandle.dismiss).not.toHaveBeenCalled(); + }); + + it("hides a dialog when saving is complete", function () { + action.perform(); + expect(dialogService.showBlockingMessage).toHaveBeenCalled(); + expect(mockDialogHandle.dismiss).toHaveBeenCalled(); + }); }); }); diff --git a/platform/commonUI/edit/test/actions/SaveAsActionSpec.js b/platform/commonUI/edit/test/actions/SaveAsActionSpec.js index 37e35aa5af..b949f6b447 100644 --- a/platform/commonUI/edit/test/actions/SaveAsActionSpec.js +++ b/platform/commonUI/edit/test/actions/SaveAsActionSpec.js @@ -101,8 +101,7 @@ define( "dialogService", [ "getUserInput", - "showBlockingMessage", - "dismiss" + "showBlockingMessage" ] ); mockDialogService.getUserInput.andReturn(mockPromise(undefined)); @@ -171,17 +170,27 @@ define( expect(mockDialogService.getUserInput).toHaveBeenCalled(); }); - it("shows a blocking dialog while waiting for save", function () { - mockEditorCapability.save.andReturn(new Promise(function () {})); - action.perform(); - expect(mockDialogService.showBlockingMessage).toHaveBeenCalled(); - expect(mockDialogService.dismiss).not.toHaveBeenCalled(); - }); + describe("a blocking dialog", function () { + var mockDialogHandle; + + beforeEach(function () { + mockDialogHandle = jasmine.createSpyObj("dialogHandle", ["dismiss"]); + mockDialogService.showBlockingMessage.andReturn(mockDialogHandle); + }); + + it("indicates that a save is taking place", function () { + mockEditorCapability.save.andReturn(new Promise(function () {})); + action.perform(); + expect(mockDialogService.showBlockingMessage).toHaveBeenCalled(); + expect(mockDialogHandle.dismiss).not.toHaveBeenCalled(); + }); + + it("is hidden after saving", function () { + action.perform(); + expect(mockDialogService.showBlockingMessage).toHaveBeenCalled(); + expect(mockDialogHandle.dismiss).toHaveBeenCalled(); + }); - it("hides the blocking dialog after saving", function () { - action.perform(); - expect(mockDialogService.showBlockingMessage).toHaveBeenCalled(); - expect(mockDialogService.dismiss).toHaveBeenCalled(); }); }); diff --git a/platform/commonUI/general/src/controllers/BannerController.js b/platform/commonUI/general/src/controllers/BannerController.js index 5b0cdd61bc..098bacfa9f 100644 --- a/platform/commonUI/general/src/controllers/BannerController.js +++ b/platform/commonUI/general/src/controllers/BannerController.js @@ -54,17 +54,17 @@ define( }; $scope.maximize = function (notification) { if (notification.model.severity !== "info") { - + var dialog; notification.model.cancel = function () { - dialogService.dismiss(); + dialog.dismiss(); }; //If the notification is dismissed by the user, close // the dialog. notification.onDismiss(function () { - dialogService.dismiss(); + dialog.dismiss(); }); - dialogService.showBlockingMessage(notification.model); + dialog = dialogService.showBlockingMessage(notification.model); } }; } diff --git a/platform/commonUI/notification/src/NotificationIndicatorController.js b/platform/commonUI/notification/src/NotificationIndicatorController.js index 8a1bdbca2a..f5343df46d 100644 --- a/platform/commonUI/notification/src/NotificationIndicatorController.js +++ b/platform/commonUI/notification/src/NotificationIndicatorController.js @@ -49,9 +49,6 @@ define( //Launch the message list dialog with the models // from the notifications messages: notificationService.notifications - }, - cancel: function () { - dialogService.dismiss(); } }); diff --git a/platform/commonUI/notification/test/NotificationIndicatorControllerSpec.js b/platform/commonUI/notification/test/NotificationIndicatorControllerSpec.js index 222c2fc87b..adcc912b60 100644 --- a/platform/commonUI/notification/test/NotificationIndicatorControllerSpec.js +++ b/platform/commonUI/notification/test/NotificationIndicatorControllerSpec.js @@ -54,22 +54,7 @@ define( expect(mockDialogService.getDialogResponse).toHaveBeenCalled(); expect(mockDialogService.getDialogResponse.mostRecentCall.args[0]).toBe('overlay-message-list'); expect(mockDialogService.getDialogResponse.mostRecentCall.args[1].dialog).toBeDefined(); - expect(mockDialogService.getDialogResponse.mostRecentCall.args[1].cancel).toBeDefined(); - //Invoke the cancel callback - mockDialogService.getDialogResponse.mostRecentCall.args[1].cancel(); - expect(mockDialogService.dismiss).toHaveBeenCalled(); }); - - it("provides a means of dismissing the message list", function () { - expect(mockScope.showNotificationsList).toBeDefined(); - mockScope.showNotificationsList(); - expect(mockDialogService.getDialogResponse).toHaveBeenCalled(); - expect(mockDialogService.getDialogResponse.mostRecentCall.args[1].cancel).toBeDefined(); - //Invoke the cancel callback - mockDialogService.getDialogResponse.mostRecentCall.args[1].cancel(); - expect(mockDialogService.dismiss).toHaveBeenCalled(); - }); - }); } ); diff --git a/platform/entanglement/src/actions/CopyAction.js b/platform/entanglement/src/actions/CopyAction.js index 0c5ab99755..ce40f3bbd2 100644 --- a/platform/entanglement/src/actions/CopyAction.js +++ b/platform/entanglement/src/actions/CopyAction.js @@ -86,7 +86,9 @@ define( severity: "info" }); } else if (phase.toLowerCase() === "copying") { - this.dialogService.dismiss(); + if (this.dialog) { + this.dialog.dismiss(); + } if (!this.notification) { this.notification = this.notificationService .notify({ @@ -115,7 +117,8 @@ define( } function error(errorDetails) { - var errorMessage = { + var errorDialog, + errorMessage = { title: "Error copying objects.", severity: "error", hint: errorDetails.message, @@ -123,12 +126,12 @@ define( options: [{ label: "OK", callback: function () { - self.dialogService.dismiss(); + errorDialog.dismiss(); } }] }; - self.dialogService.dismiss(); + self.dialog.dismiss(); if (self.notification) { self.notification.dismiss(); // Clear the progress notification } @@ -136,7 +139,7 @@ define( //Show a minimized notification of error for posterity self.notificationService.notify(errorMessage); //Display a blocking message - self.dialogService.showBlockingMessage(errorMessage); + errorDialog = self.dialogService.showBlockingMessage(errorMessage); } function notification(details) { diff --git a/platform/entanglement/test/actions/CopyActionSpec.js b/platform/entanglement/test/actions/CopyActionSpec.js index 176b5baef8..8d9b6397b6 100644 --- a/platform/entanglement/test/actions/CopyActionSpec.js +++ b/platform/entanglement/test/actions/CopyActionSpec.js @@ -44,6 +44,7 @@ define( notificationService, notification, dialogService, + mockDialog, mockLog, abstractComposePromise, progress = {phase: "copying", totalObjects: 10, processed: 1}; @@ -120,9 +121,12 @@ define( .andReturn(locationServicePromise); dialogService = jasmine.createSpyObj('dialogService', - ['showBlockingMessage', 'dismiss'] + ['showBlockingMessage'] ); + mockDialog = jasmine.createSpyObj("dialog", ["dismiss"]); + dialogService.showBlockingMessage.andReturn(mockDialog); + notification = jasmine.createSpyObj('notification', ['dismiss', 'model'] );