From f2d61604f7bad162d232d18f933029cf7f90bf58 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 15:07:05 -0800 Subject: [PATCH] [Navigation] navigationService provides checking Remove policy checking in navigation action and depend on navigation service to provide those checks. * Register checkFunctions with navigationService.checkBeforeNavigation which returns a function for unregistering them. * navigationService.setNavigation will run checks before allowing navigation, unless a `force` argument is supplied. https://github.com/nasa/openmct/issues/1360 --- platform/commonUI/browse/bundle.js | 10 +- .../browse/src/navigation/NavigateAction.js | 33 +----- .../src/navigation/NavigationService.js | 106 ++++++++++++++++-- 3 files changed, 108 insertions(+), 41 deletions(-) diff --git a/platform/commonUI/browse/bundle.js b/platform/commonUI/browse/bundle.js index 0dd556c40c..3d3d76ee8e 100644 --- a/platform/commonUI/browse/bundle.js +++ b/platform/commonUI/browse/bundle.js @@ -198,7 +198,10 @@ define([ "services": [ { "key": "navigationService", - "implementation": NavigationService + "implementation": NavigationService, + "depends": [ + "$window" + ] } ], "actions": [ @@ -206,10 +209,7 @@ define([ "key": "navigate", "implementation": NavigateAction, "depends": [ - "navigationService", - "$q", - "policyService", - "$window" + "navigationService" ] }, { diff --git a/platform/commonUI/browse/src/navigation/NavigateAction.js b/platform/commonUI/browse/src/navigation/NavigateAction.js index 6121213a51..775f00c8bb 100644 --- a/platform/commonUI/browse/src/navigation/NavigateAction.js +++ b/platform/commonUI/browse/src/navigation/NavigateAction.js @@ -33,12 +33,9 @@ define( * @constructor * @implements {Action} */ - function NavigateAction(navigationService, $q, policyService, $window, context) { + function NavigateAction(navigationService, context) { this.domainObject = context.domainObject; - this.$q = $q; this.navigationService = navigationService; - this.policyService = policyService; - this.$window = $window; } /** @@ -51,32 +48,12 @@ define( navigateTo = this.domainObject, currentObject = self.navigationService.getNavigation(); - function allow() { - var navigationAllowed = true; - self.policyService.allow("navigation", currentObject, navigateTo, function (message) { - navigationAllowed = self.$window.confirm(message + "\r\n\r\n" + - " Are you sure you want to continue?"); - }); - return navigationAllowed; - } - - function cancelIfEditing() { - var editing = currentObject.hasCapability('editor') && - currentObject.getCapability('editor').isEditContextRoot(); - - return self.$q.when(editing && currentObject.getCapability("editor").finish()); - } - - function navigate() { - return self.navigationService.setNavigation(navigateTo); - } - - if (allow()) { - return cancelIfEditing().then(navigate); - } else { - return this.$q.when(false); + if (this.navigationService.shouldNavigate()) { + this.navigationService.setNavigation(this.domainObject, true); + return Promise.resolve({}); } + return Promise.reject('Navigation Prevented by User'); }; /** diff --git a/platform/commonUI/browse/src/navigation/NavigationService.js b/platform/commonUI/browse/src/navigation/NavigationService.js index 2791a6e9b0..a3307d5276 100644 --- a/platform/commonUI/browse/src/navigation/NavigationService.js +++ b/platform/commonUI/browse/src/navigation/NavigationService.js @@ -30,16 +30,20 @@ define( /** * The navigation service maintains the application's current * navigation state, and allows listening for changes thereto. + * * @memberof platform/commonUI/browse * @constructor */ - function NavigationService() { + function NavigationService($window) { this.navigated = undefined; this.callbacks = []; + this.checks = []; + this.$window = $window; } /** * Get the current navigation state. + * * @returns {DomainObject} the object that is navigated-to */ NavigationService.prototype.getNavigation = function () { @@ -47,16 +51,33 @@ define( }; /** - * Set the current navigation state. This will invoke listeners. + * Navigate to a specified object. If navigation checks exist and + * return reasons to prevent navigation, it will prompt the user before + * continuing. Trying to navigate to the currently navigated object will + * do nothing. + * + * If a truthy value is passed for `force`, it will skip navigation + * and will not prevent navigation to an already selected object. + * * @param {DomainObject} domainObject the domain object to navigate to + * @param {Boolean} force if true, force navigation to occur. + * @returns {Boolean} true if navigation occured, otherwise false. */ - NavigationService.prototype.setNavigation = function (value) { - if (this.navigated !== value) { - this.navigated = value; - this.callbacks.forEach(function (callback) { - callback(value); - }); + NavigationService.prototype.setNavigation = function (domainObject, force) { + if (force) { + this.doNavigation(domainObject); + return true; } + if (this.navigated === domainObject) { + return true; + } + + var doNotNavigate = this.shouldWarnBeforeNavigate(); + if (doNotNavigate && !this.$window.confirm(doNotNavigate)) { + return false; + } + + this.doNavigation(domainObject); return true; }; @@ -64,6 +85,7 @@ define( * Listen for changes in navigation. The passed callback will * be invoked with the new domain object of navigation when * this changes. + * * @param {function} callback the callback to invoke when * navigation state changes */ @@ -73,6 +95,7 @@ define( /** * Stop listening for changes in navigation state. + * * @param {function} callback the callback which should * no longer be invoked when navigation state * changes @@ -83,6 +106,73 @@ define( }); }; + /** + * Check if navigation should proceed. May prompt a user for input + * if any checkFns return messages. Returns true if the user wishes to + * navigate, otherwise false. If using this prior to calling + * `setNavigation`, you should call `setNavigation` with `force=true` + * to prevent duplicate dialogs being displayed to the user. + * + * @returns {Boolean} true if the user wishes to navigate, otherwise false. + */ + NavigationService.prototype.shouldNavigate = function () { + var doNotNavigate = this.shouldWarnBeforeNavigate(); + return !doNotNavigate || this.$window.confirm(doNotNavigate); + }; + + /** + * Register a check function to be called before any navigation occurs. + * Check functions should return a human readable "message" if + * there are any reasons to prevent navigation. Otherwise, they should + * return falsy. Returns a function which can be called to remove the + * check function. + * + * @param {Function} checkFn a function to call before navigation occurs. + * @returns {Function} removeCheck call to remove check + */ + NavigationService.prototype.checkBeforeNavigation = function (checkFn) { + this.checks.push(checkFn); + return function removeCheck() { + this.checks = this.checks.filter(function (fn) { + return checkFn !== fn; + }); + }.bind(this); + }; + + /** + * Private method to actually perform navigation. + * + * @private + */ + NavigationService.prototype.doNavigation = function (value) { + this.navigated = value; + this.callbacks.forEach(function (callback) { + callback(value); + }); + }; + + /** + * Returns either a false value, or a string that should be displayed + * to the user before navigation is allowed. + * + * @private + */ + NavigationService.prototype.shouldWarnBeforeNavigate = function () { + var reasons = []; + + this.checks.forEach(function (checkFn) { + var reason = checkFn(); + if (reason) { + reasons.push(reason); + } + }); + + if (reasons.length) { + return reasons.join('\n'); + } + return false; + }; + return NavigationService; } );