Merge pull request nasa#1366 from nasa/fix-navigation-warnings-1360

Fix navigation warnings, tidy editing, navigation, browse logic.

Author: VWoeltjen

docs/src/guide/index.md

@@ -1338,20 +1338,6 @@ are supported:
 
 Open MCT defines several Angular directives that are intended for use both 
 internally within the platform, and by plugins. 
-
-## Before Unload 
-
-The `mct-before-unload` directive is used to listen for (and prompt for user 
-confirmation) of navigation changes in the browser. This includes reloading, 
-following links out of Open MCT, or changing routes. It is used to hook into 
-both `onbeforeunload` event handling as well as route changes from within 
-Angular.
-
-This directive is useable as an attribute. Its value should be an Angular 
-expression. When an action that would trigger an unload and/or route change 
-occurs, this Angular expression is evaluated. Its result should be a message to 
-display to the user to confirm their navigation change; if this expression 
-evaluates to a falsy value, no message will be displayed. 
 
 ## Chart 
 

platform/commonUI/browse/bundle.js

@@ -41,7 +41,6 @@ define([
     "text!./res/templates/items/items.html",
     "text!./res/templates/browse/object-properties.html",
     "text!./res/templates/browse/inspector-region.html",
-    "text!./res/templates/view-object.html",
     'legacyRegistry'
 ], function (
     BrowseController,
@@ -64,7 +63,6 @@ define([
     itemsTemplate,
     objectPropertiesTemplate,
     inspectorRegionTemplate,
-    viewObjectTemplate,
     legacyRegistry
 ) {
 
@@ -141,10 +139,6 @@ define([
                 }
             ],
             "representations": [
-                {
-                    "key": "view-object",
-                    "template": viewObjectTemplate
-                },
                 {
                     "key": "browse-object",
                     "template": browseObjectTemplate,
@@ -204,18 +198,18 @@ define([
             "services": [
                 {
                     "key": "navigationService",
-                    "implementation": NavigationService
+                    "implementation": NavigationService,
+                    "depends": [
+                        "$window"
+                    ]
                 }
             ],
             "actions": [
                 {
                     "key": "navigate",
                     "implementation": NavigateAction,
                     "depends": [
-                        "navigationService",
-                        "$q",
-                        "policyService",
-                        "$window"
+                        "navigationService"
                     ]
                 },
                 {

platform/commonUI/browse/res/templates/browse.html

@@ -63,7 +63,7 @@
                     <mct-split-pane class='l-object-and-inspector contents abs' anchor='right'>
                         <div class='split-pane-component t-object pane primary-pane left'>
                             <mct-representation mct-object="navigatedObject"
-                                                key="'view-object'"
+                                                key="navigatedObject.getCapability('status').get('editing') ? 'edit-object' : 'browse-object'"
                                                 class="abs holder holder-object">
                             </mct-representation>
                             <a class="mini-tab-icon anchor-right mobile-hide toggle-pane toggle-inspect flush-right"

platform/commonUI/browse/res/templates/view-object.html

@@ -48,11 +48,16 @@ define(
             defaultPath
         ) {
             var initialPath = ($route.current.params.ids || defaultPath).split("/");
-
-            var currentIds = $route.current.params.ids;
+            var currentIds;
 
             $scope.treeModel = {
-                selectedObject: undefined
+                selectedObject: undefined,
+                onSelection: function (object) {
+                    navigationService.setNavigation(object, true);
+                },
+                allowSelection: function (object) {
+                    return navigationService.shouldNavigate();
+                }
             };
 
             function idsForObject(domainObject) {
@@ -103,7 +108,6 @@ define(
             function navigateToObject(desiredObject) {
                 $scope.navigatedObject = desiredObject;
                 $scope.treeModel.selectedObject = desiredObject;
-                navigationService.setNavigation(desiredObject);
                 currentIds = idsForObject(desiredObject);
                 $route.current.pathParams.ids = currentIds;
                 $location.path('/browse/' + currentIds);
@@ -114,10 +118,11 @@ define(
                     .then(function (root) {
                         return findViaComposition(root, path);
                     })
-                    .then(navigateToObject);
+                    .then(function (object) {
+                        navigationService.setNavigation(object);
+                    });
             }
 
-
             getObject('ROOT')
                 .then(function (root) {
                     $scope.domainObject = root;
@@ -137,15 +142,6 @@ define(
             // Listen for changes in navigation state.
             navigationService.addListener(navigateDirectlyToModel);
 
-            // Also listen for changes which come from the tree. Changes in
-            // the tree will trigger a change in browse navigation state.
-            $scope.$watch("treeModel.selectedObject", function (newObject, oldObject) {
-                if (oldObject !== newObject) {
-                    navigateDirectlyToModel(newObject);
-                }
-            });
-
-
             // Listen for route changes which are caused by browser events
             // (e.g. bookmarks to pages in OpenMCT) and prevent them.  Instead,
             // navigate to the path ourselves, which results in it being

platform/commonUI/browse/src/BrowseController.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;
         }
 
         /**
@@ -47,36 +44,12 @@ define(
          *          navigation has been updated
          */
         NavigateAction.prototype.perform = function () {
-            var self = this,
-                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');
         };
 
         /**

platform/commonUI/browse/src/navigation/NavigateAction.js

@@ -30,40 +30,65 @@ 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;
+
+            this.oldUnload = $window.onbeforeunload;
+            $window.onbeforeunload = this.onBeforeUnload.bind(this);
         }
 
         /**
          * Get the current navigation state.
+         *
          * @returns {DomainObject} the object that is navigated-to
          */
         NavigationService.prototype.getNavigation = function () {
             return this.navigated;
         };
 
         /**
-         * 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;
         };
 
         /**
          * 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 +98,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 +109,89 @@ 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;
+        };
+
+        /**
+         * Listener for window on before unload event-- will warn before
+         * navigation is allowed.
+         *
+         * @private
+         */
+        NavigationService.prototype.onBeforeUnload = function () {
+            var shouldWarnBeforeNavigate = this.shouldWarnBeforeNavigate();
+            if (shouldWarnBeforeNavigate) {
+                return shouldWarnBeforeNavigate;
+            }
+            if (this.oldUnload) {
+                return this.oldUnload.apply(undefined, [].slice.apply(arguments));
+            }
+        };
+
         return NavigationService;
     }
 );