From 0b79ec1235952d8dd7c350d9d45755133bb4c692 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 14:45:08 -0800 Subject: [PATCH 1/9] [Browse] Simplify Edit Representation Simplify edit registration and remove extra abstractions. No longer attach a status listener for every representation-- just use a single watch for the edit controller. Simplifies logic involved in switching controllers. https://github.com/nasa/openmct/issues/1360 --- platform/commonUI/browse/bundle.js | 6 --- .../commonUI/browse/res/templates/browse.html | 2 +- .../browse/res/templates/view-object.html | 33 --------------- .../edit/src/representers/EditRepresenter.js | 41 +------------------ 4 files changed, 3 insertions(+), 79 deletions(-) delete mode 100644 platform/commonUI/browse/res/templates/view-object.html diff --git a/platform/commonUI/browse/bundle.js b/platform/commonUI/browse/bundle.js index bed2fd1fbd..0dd556c40c 100644 --- a/platform/commonUI/browse/bundle.js +++ b/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, diff --git a/platform/commonUI/browse/res/templates/browse.html b/platform/commonUI/browse/res/templates/browse.html index aa1415ca81..214c765add 100644 --- a/platform/commonUI/browse/res/templates/browse.html +++ b/platform/commonUI/browse/res/templates/browse.html @@ -63,7 +63,7 @@
- - - diff --git a/platform/commonUI/edit/src/representers/EditRepresenter.js b/platform/commonUI/edit/src/representers/EditRepresenter.js index 81252977b1..fa08d7be7b 100644 --- a/platform/commonUI/edit/src/representers/EditRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditRepresenter.js @@ -47,7 +47,6 @@ define( var self = this; this.scope = scope; - this.listenHandle = undefined; // Mutate and persist a new version of a domain object's model. function doMutate(model) { @@ -90,51 +89,15 @@ define( // Place the "commit" method in the scope scope.commit = commit; - // Clean up when the scope is destroyed - scope.$on("$destroy", function () { - self.destroy(); - }); - } // Handle a specific representation of a specific domain object - EditRepresenter.prototype.represent = function represent(representation, representedObject) { - var scope = this.scope; - - // Track the key, to know which view configuration to save to. - this.key = (representation || {}).key; - // Track the represented object + EditRepresenter.prototype.represent = function (representation, representedObject) { this.domainObject = representedObject; - - // Ensure existing watches are released - this.destroy(); - - function setEditing() { - scope.viewObjectTemplate = 'edit-object'; - } - - /** - * Listen for changes in object state. If the object becomes - * editable then change the view and inspector regions - * object representation accordingly - */ - this.listenHandle = this.domainObject.getCapability('status').listen(function (statuses) { - if (statuses.indexOf('editing') !== -1) { - setEditing(); - } else { - delete scope.viewObjectTemplate; - } - }); - - if (representedObject.hasCapability('editor') && representedObject.getCapability('editor').isEditContextRoot()) { - setEditing(); - } }; // Respond to the destruction of the current representation. - EditRepresenter.prototype.destroy = function destroy() { - return this.listenHandle && this.listenHandle(); - }; + EditRepresenter.prototype.destroy = function () {}; return EditRepresenter; } From f0b9292458dbdc6f29eb409500328eb1b223fde1 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 14:48:36 -0800 Subject: [PATCH 2/9] [Tree] Add additional api methods Add methods to tree view via scope for more fine grained control. Can supply a "allowSelection" function which should return true if a given selection is allowed. This is executed before a node is selected and allows you to prevent selection. Before this, if you wanted to prevent the selection from changing, you had to wait for it to change and then change it back to the original value. Can also supply an "onSelection" function which is called when a value is successfully selected. This allows you to handle the selection event without waiting for digest. You can still $watch "selectedObject" if you prefer. Additionally, this changes the tree node to trigger a digest only when the value is set via a MouseClick, instead of every time. Tidies up directive scope bindings to clarify usage. https://github.com/nasa/openmct/issues/1360 --- .../general/res/templates/subtree.html | 5 +- .../general/src/directives/MCTTree.js | 49 +++++++++++++++---- .../commonUI/general/src/ui/TreeNodeView.js | 4 +- platform/commonUI/general/src/ui/TreeView.js | 4 +- 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/platform/commonUI/general/res/templates/subtree.html b/platform/commonUI/general/res/templates/subtree.html index d9d7481d59..1290d17f8a 100644 --- a/platform/commonUI/general/res/templates/subtree.html +++ b/platform/commonUI/general/res/templates/subtree.html @@ -19,6 +19,9 @@ this source code distribution or the Licensing information page available at runtime from the About dialog for additional information. --> - + diff --git a/platform/commonUI/general/src/directives/MCTTree.js b/platform/commonUI/general/src/directives/MCTTree.js index 3b08691125..cae72d497b 100644 --- a/platform/commonUI/general/src/directives/MCTTree.js +++ b/platform/commonUI/general/src/directives/MCTTree.js @@ -26,25 +26,54 @@ define([ ], function (angular, TreeView) { function MCTTree(gestureService) { function link(scope, element) { - var treeView = new TreeView(gestureService), - unobserve = treeView.observe(function (domainObject) { - if (scope.mctModel !== domainObject) { - scope.mctModel = domainObject; - scope.$apply(); - } - }); + if (!scope.allowSelection) { + scope.allowSelection = function () { + return true; + } + } + if (!scope.onSelection) { + scope.onSelection = function () {}; + } + var currentSelection = scope.selectedObject; + var treeView = new TreeView(gestureService); + + function setSelection(domainObject, event) { + if (currentSelection === domainObject) { + return; + } + if (!scope.allowSelection(domainObject)) { + treeView.value(currentSelection); + return; + } + currentSelection = domainObject; + scope.onSelection(domainObject); + scope.selectedObject = domainObject; + if (event && event instanceof MouseEvent) { + scope.$apply(); + } + } + + var unobserve = treeView.observe(setSelection); element.append(angular.element(treeView.elements())); - scope.$watch('mctModel', treeView.value.bind(treeView)); - scope.$watch('mctObject', treeView.model.bind(treeView)); + scope.$watch('selectedObject', function (object) { + currentSelection = object; + treeView.value(object); + }); + scope.$watch('rootObject', treeView.model.bind(treeView)); scope.$on('$destroy', unobserve); } return { restrict: "E", link: link, - scope: { mctObject: "=", mctModel: "=" } + scope: { + rootObject: "=", + selectedObject: "=", + onSelection: "=?", + allowSelection: "=?" + } }; } diff --git a/platform/commonUI/general/src/ui/TreeNodeView.js b/platform/commonUI/general/src/ui/TreeNodeView.js index 41430e5aa1..993316c4da 100644 --- a/platform/commonUI/general/src/ui/TreeNodeView.js +++ b/platform/commonUI/general/src/ui/TreeNodeView.js @@ -49,8 +49,8 @@ define([ this.labelView = new TreeLabelView(gestureService); - $(this.labelView.elements()).on('click', function () { - selectFn(this.activeObject); + $(this.labelView.elements()).on('click', function (event) { + selectFn(this.activeObject, event); }.bind(this)); this.li.append($(nodeTemplate)); diff --git a/platform/commonUI/general/src/ui/TreeView.js b/platform/commonUI/general/src/ui/TreeView.js index 92719d6113..f0cb0b16ad 100644 --- a/platform/commonUI/general/src/ui/TreeView.js +++ b/platform/commonUI/general/src/ui/TreeView.js @@ -109,11 +109,11 @@ define([ }.bind(this)); }; - TreeView.prototype.value = function (domainObject) { + TreeView.prototype.value = function (domainObject, event) { this.selectedObject = domainObject; this.updateNodeViewSelection(); this.callbacks.forEach(function (callback) { - callback(domainObject); + callback(domainObject, event); }); }; From f2d61604f7bad162d232d18f933029cf7f90bf58 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 15:07:05 -0800 Subject: [PATCH 3/9] [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; } ); From daa71c4f69ac3d1e73ada1c4a43ccfd615243199 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 15:10:07 -0800 Subject: [PATCH 4/9] [Navigation] remove mct-before-unload Remove mct-before-unload, and move the functionality to the navigation service. The navigation service considers "unload" to be a navigation event and prompts in much the same way as it would before any other navigation event. https://github.com/nasa/openmct/issues/1360 --- docs/src/guide/index.md | 14 --- .../src/navigation/NavigationService.js | 19 +++ platform/commonUI/edit/README.md | 19 --- platform/commonUI/edit/bundle.js | 11 -- .../edit/res/templates/edit-object.html | 3 +- .../src/controllers/EditObjectController.js | 18 --- .../edit/src/directives/MCTBeforeUnload.js | 104 ---------------- .../test/directives/MCTBeforeUnloadSpec.js | 114 ------------------ 8 files changed, 20 insertions(+), 282 deletions(-) delete mode 100644 platform/commonUI/edit/src/directives/MCTBeforeUnload.js delete mode 100644 platform/commonUI/edit/test/directives/MCTBeforeUnloadSpec.js diff --git a/docs/src/guide/index.md b/docs/src/guide/index.md index 1d8a422299..c4d52c9501 100644 --- a/docs/src/guide/index.md +++ b/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 diff --git a/platform/commonUI/browse/src/navigation/NavigationService.js b/platform/commonUI/browse/src/navigation/NavigationService.js index a3307d5276..0cc520e757 100644 --- a/platform/commonUI/browse/src/navigation/NavigationService.js +++ b/platform/commonUI/browse/src/navigation/NavigationService.js @@ -39,6 +39,9 @@ define( this.callbacks = []; this.checks = []; this.$window = $window; + + this.oldUnload = $window.onbeforeunload; + $window.onbeforeunload = this.onBeforeUnload.bind(this); } /** @@ -173,6 +176,22 @@ define( 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; } ); diff --git a/platform/commonUI/edit/README.md b/platform/commonUI/edit/README.md index a1e88d330c..ca52b78f1a 100644 --- a/platform/commonUI/edit/README.md +++ b/platform/commonUI/edit/README.md @@ -2,25 +2,6 @@ Contains sources and resources associated with Edit mode. # Extensions -## Directives - -This bundle introduces the `mct-before-unload` directive, primarily for -internal use (to prompt the user to confirm navigation away from unsaved -changes in Edit mode.) - -The `mct-before-unload` directive is used as an attribute whose value is -an Angular expression that is evaluated when navigation changes (either -via browser-level changes, such as the refresh button, or changes to -the Angular route, which happens when hitting the back button in Edit -mode.) The result of this evaluation, when truthy, is shown in a browser -dialog to allow the user to confirm navigation. When falsy, no prompt is -shown, allowing these dialogs to be shown conditionally. (For instance, in -Edit mode, prompts are only shown if user-initiated changes have -occurred.) - -This directive may be attached to any element; its behavior will be enforced -so long as that element remains within the DOM. - # Toolbars Views may specify the contents of a toolbar through a `toolbar` diff --git a/platform/commonUI/edit/bundle.js b/platform/commonUI/edit/bundle.js index 8b9aad6d21..d60afc9add 100644 --- a/platform/commonUI/edit/bundle.js +++ b/platform/commonUI/edit/bundle.js @@ -25,7 +25,6 @@ define([ "./src/controllers/EditPanesController", "./src/controllers/ElementsController", "./src/controllers/EditObjectController", - "./src/directives/MCTBeforeUnload", "./src/actions/EditAndComposeAction", "./src/actions/EditAction", "./src/actions/PropertiesAction", @@ -65,7 +64,6 @@ define([ EditPanesController, ElementsController, EditObjectController, - MCTBeforeUnload, EditAndComposeAction, EditAction, PropertiesAction, @@ -152,15 +150,6 @@ define([ ] } ], - "directives": [ - { - "key": "mctBeforeUnload", - "implementation": MCTBeforeUnload, - "depends": [ - "$window" - ] - } - ], "actions": [ { "key": "compose", diff --git a/platform/commonUI/edit/res/templates/edit-object.html b/platform/commonUI/edit/res/templates/edit-object.html index f93f3cd60f..dcd892c9d3 100644 --- a/platform/commonUI/edit/res/templates/edit-object.html +++ b/platform/commonUI/edit/res/templates/edit-object.html @@ -20,8 +20,7 @@ at runtime from the About dialog for additional information. -->
-
+
Date: Tue, 20 Dec 2016 15:14:42 -0800 Subject: [PATCH 5/9] [Edit] manage editing in EditObjectController EditObjectController now exits edit mode when it is destroyed. It also injects a check function in the navigation service to replace the old functionality implemented in EditNavigationPolicy. https://github.com/nasa/openmct/issues/1360 --- platform/commonUI/edit/bundle.js | 9 +-- .../src/controllers/EditObjectController.js | 40 ++++++++++-- .../edit/src/policies/EditNavigationPolicy.js | 64 ------------------- 3 files changed, 36 insertions(+), 77 deletions(-) delete mode 100644 platform/commonUI/edit/src/policies/EditNavigationPolicy.js diff --git a/platform/commonUI/edit/bundle.js b/platform/commonUI/edit/bundle.js index d60afc9add..6355ea9b8a 100644 --- a/platform/commonUI/edit/bundle.js +++ b/platform/commonUI/edit/bundle.js @@ -36,7 +36,6 @@ define([ "./src/policies/EditActionPolicy", "./src/policies/EditableLinkPolicy", "./src/policies/EditableMovePolicy", - "./src/policies/EditNavigationPolicy", "./src/policies/EditContextualActionPolicy", "./src/representers/EditRepresenter", "./src/representers/EditToolbarRepresenter", @@ -75,7 +74,6 @@ define([ EditActionPolicy, EditableLinkPolicy, EditableMovePolicy, - EditNavigationPolicy, EditContextualActionPolicy, EditRepresenter, EditToolbarRepresenter, @@ -130,7 +128,7 @@ define([ "depends": [ "$scope", "$location", - "policyService" + "navigationService" ] }, { @@ -262,11 +260,6 @@ define([ "category": "action", "implementation": EditableLinkPolicy }, - { - "category": "navigation", - "message": "Continuing will cause the loss of any unsaved changes.", - "implementation": EditNavigationPolicy - }, { "implementation": CreationPolicy, "category": "creation" diff --git a/platform/commonUI/edit/src/controllers/EditObjectController.js b/platform/commonUI/edit/src/controllers/EditObjectController.js index 219faa3838..e2d4b5e60a 100644 --- a/platform/commonUI/edit/src/controllers/EditObjectController.js +++ b/platform/commonUI/edit/src/controllers/EditObjectController.js @@ -28,17 +28,48 @@ define( [], function () { + function isDirty(domainObject) { + var navigatedObject = domainObject, + editorCapability = navigatedObject && + navigatedObject.getCapability("editor"); + + return editorCapability && + editorCapability.isEditContextRoot() && + editorCapability.dirty(); + } + + function cancelEditing(domainObject) { + var navigatedObject = domainObject, + editorCapability = navigatedObject && + navigatedObject.getCapability("editor"); + + return editorCapability && + editorCapability.finish(); + } + /** * Controller which is responsible for populating the scope for * Edit mode * @memberof platform/commonUI/edit * @constructor */ - function EditObjectController($scope, $location, policyService) { + function EditObjectController($scope, $location, navigationService) { this.scope = $scope; - this.policyService = policyService; + var domainObject = $scope.domainObject; + + var removeCheck = navigationService + .checkBeforeNavigation(function () { + if (isDirty(domainObject)) { + return "Continuing will cause the loss of any unsaved changes."; + } + return false; + }); + + $scope.$on('$destroy', function () { + removeCheck(); + cancelEditing(domainObject); + }); - var navigatedObject; function setViewForDomainObject(domainObject) { var locationViewKey = $location.search().view; @@ -54,10 +85,9 @@ define( ((domainObject && domainObject.useCapability('view')) || []) .forEach(selectViewIfMatching); } - navigatedObject = domainObject; } - $scope.$watch('domainObject', setViewForDomainObject); + setViewForDomainObject($scope.domainObject); $scope.doAction = function (action) { return $scope[action] && $scope[action](); diff --git a/platform/commonUI/edit/src/policies/EditNavigationPolicy.js b/platform/commonUI/edit/src/policies/EditNavigationPolicy.js deleted file mode 100644 index fc5fcd64b7..0000000000 --- a/platform/commonUI/edit/src/policies/EditNavigationPolicy.js +++ /dev/null @@ -1,64 +0,0 @@ -/***************************************************************************** - * Open MCT, Copyright (c) 2014-2016, United States Government - * as represented by the Administrator of the National Aeronautics and Space - * Administration. All rights reserved. - * - * Open MCT is licensed under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0. - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - * - * Open MCT includes source code licensed under additional open source - * licenses. See the Open Source Licenses file (LICENSES.md) included with - * this source code distribution or the Licensing information page available - * at runtime from the About dialog for additional information. - *****************************************************************************/ - -define( - [], - function () { - - /** - * Policy controlling whether navigation events should proceed - * when object is being edited. - * @memberof platform/commonUI/edit - * @constructor - * @implements {Policy.} - */ - function EditNavigationPolicy(policyService) { - this.policyService = policyService; - } - - /** - * @private - */ - EditNavigationPolicy.prototype.isDirty = function (domainObject) { - var navigatedObject = domainObject, - editorCapability = navigatedObject && - navigatedObject.getCapability("editor"); - - return editorCapability && - editorCapability.isEditContextRoot() && - editorCapability.dirty(); - }; - - /** - * Allow navigation if an object is not dirty, or if the user elects - * to proceed anyway. - * @param currentNavigation - * @returns {boolean|*} true if the object model is clean; or if - * it's dirty and the user wishes to proceed anyway. - */ - EditNavigationPolicy.prototype.allow = function (currentNavigation) { - return !this.isDirty(currentNavigation); - }; - - return EditNavigationPolicy; - } -); From 6328bd9354c40dfb4bb4a7b7261a4c77a8117e2a Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 15:28:15 -0800 Subject: [PATCH 6/9] [Edit] Cancel action depends on promise resolution Cancel action no longer cares about return value, simply will not execute if navigtion promise does not resolve. https://github.com/nasa/openmct/issues/1360 --- platform/commonUI/edit/src/actions/CancelAction.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/platform/commonUI/edit/src/actions/CancelAction.js b/platform/commonUI/edit/src/actions/CancelAction.js index a48ceec996..474939cb1d 100644 --- a/platform/commonUI/edit/src/actions/CancelAction.js +++ b/platform/commonUI/edit/src/actions/CancelAction.js @@ -56,13 +56,13 @@ define( //navigate back to parent because nothing to show. return domainObject.getCapability("location").getOriginal().then(function (original) { parent = original.getCapability("context").getParent(); - parent.getCapability("action").perform("navigate"); + return parent.getCapability("action").perform("navigate"); }); } } - function cancel(allowed) { - return allowed && domainObject.getCapability("editor").finish(); + function cancel() { + return domainObject.getCapability("editor").finish(); } //Do navigation first in order to trigger unsaved changes dialog From 89be1c810a944bf172c2dbb488186070d3dd818b Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 15:29:00 -0800 Subject: [PATCH 7/9] [Browse] tighter tree view integration BrowseController uses new MCTTreeView parameters to prevent navigation events before triggering digests. It also no longer sets navigation more frequently than it should. https://github.com/nasa/openmct/issues/1360 --- .../commonUI/browse/src/BrowseController.js | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/platform/commonUI/browse/src/BrowseController.js b/platform/commonUI/browse/src/BrowseController.js index 785a950891..9442ba9fca 100644 --- a/platform/commonUI/browse/src/BrowseController.js +++ b/platform/commonUI/browse/src/BrowseController.js @@ -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 From 96c054415d80ea5e5a4065f913bae3e3b9f2719e Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 15:30:42 -0800 Subject: [PATCH 8/9] [Cleanup] Remove unused template in adapter --- src/MCT.js | 2 - .../templates/edit-object-replacement.html | 46 ------------------- 2 files changed, 48 deletions(-) delete mode 100644 src/adapter/templates/edit-object-replacement.html diff --git a/src/MCT.js b/src/MCT.js index 32380baf88..3f55c1757d 100644 --- a/src/MCT.js +++ b/src/MCT.js @@ -25,7 +25,6 @@ define([ 'legacyRegistry', 'uuid', './api/api', - 'text!./adapter/templates/edit-object-replacement.html', './selection/Selection', './api/objects/object-utils', './ui/ViewRegistry' @@ -34,7 +33,6 @@ define([ legacyRegistry, uuid, api, - editObjectTemplate, Selection, objectUtils, ViewRegistry diff --git a/src/adapter/templates/edit-object-replacement.html b/src/adapter/templates/edit-object-replacement.html deleted file mode 100644 index f8fc33ca07..0000000000 --- a/src/adapter/templates/edit-object-replacement.html +++ /dev/null @@ -1,46 +0,0 @@ -
-
-
- - - -
-
- - - - - -
-
-
-
- -
- - - - -
- - -
-
-
From 60d1b7316062f879e1d676e6279b6b2d8161b717 Mon Sep 17 00:00:00 2001 From: Pete Richards Date: Tue, 20 Dec 2016 16:42:12 -0800 Subject: [PATCH 9/9] Update tests and correct style Update tests to reflect new functionality. Closes https://github.com/nasa/openmct/issues/1360 --- .../browse/src/navigation/NavigateAction.js | 4 - .../browse/test/BrowseControllerSpec.js | 31 +-- .../test/navigation/NavigateActionSpec.js | 197 ++++++------------ .../test/navigation/NavigationServiceSpec.js | 6 +- .../src/controllers/EditObjectController.js | 4 +- ...lerSpec.js => EditObjectControllerSpec.js} | 115 +++++----- .../test/representers/EditRepresenterSpec.js | 122 ----------- .../general/src/directives/MCTTree.js | 2 +- .../general/test/directives/MCTTreeSpec.js | 38 +++- .../commonUI/general/test/ui/TreeViewSpec.js | 5 +- 10 files changed, 179 insertions(+), 345 deletions(-) rename platform/commonUI/edit/test/controllers/{EditControllerSpec.js => EditObjectControllerSpec.js} (61%) delete mode 100644 platform/commonUI/edit/test/representers/EditRepresenterSpec.js diff --git a/platform/commonUI/browse/src/navigation/NavigateAction.js b/platform/commonUI/browse/src/navigation/NavigateAction.js index 775f00c8bb..1da83307e5 100644 --- a/platform/commonUI/browse/src/navigation/NavigateAction.js +++ b/platform/commonUI/browse/src/navigation/NavigateAction.js @@ -44,10 +44,6 @@ define( * navigation has been updated */ NavigateAction.prototype.perform = function () { - var self = this, - navigateTo = this.domainObject, - currentObject = self.navigationService.getNavigation(); - if (this.navigationService.shouldNavigate()) { this.navigationService.setNavigation(this.domainObject, true); return Promise.resolve({}); diff --git a/platform/commonUI/browse/test/BrowseControllerSpec.js b/platform/commonUI/browse/test/BrowseControllerSpec.js index 18a59f0ea9..42490b7de4 100644 --- a/platform/commonUI/browse/test/BrowseControllerSpec.js +++ b/platform/commonUI/browse/test/BrowseControllerSpec.js @@ -24,8 +24,14 @@ * MCTRepresentationSpec. Created by vwoeltje on 11/6/14. */ define( - ["../src/BrowseController"], - function (BrowseController) { + [ + "../src/BrowseController", + "../src/navigation/NavigationService" + ], + function ( + BrowseController, + NavigationService + ) { describe("The browse controller", function () { var mockScope, @@ -44,7 +50,7 @@ define( function waitsForNavigation() { var calls = mockNavigationService.setNavigation.calls.length; waitsFor(function () { - return mockNavigationService.setNavigation.calls.length > calls ; + return mockNavigationService.setNavigation.calls.length > calls; }); } @@ -92,15 +98,16 @@ define( "objectService", ["getObjects"] ); - mockNavigationService = jasmine.createSpyObj( - "navigationService", - [ - "getNavigation", - "setNavigation", - "addListener", - "removeListener" - ] - ); + mockNavigationService = new NavigationService({}); + [ + "getNavigation", + "setNavigation", + "addListener", + "removeListener" + ].forEach(function (method) { + spyOn(mockNavigationService, method) + .andCallThrough(); + }); mockRootObject = jasmine.createSpyObj( "rootObjectContainer", ["getId", "getCapability", "getModel", "useCapability", "hasCapability"] diff --git a/platform/commonUI/browse/test/navigation/NavigateActionSpec.js b/platform/commonUI/browse/test/navigation/NavigateActionSpec.js index f9b756eeaa..1299a0021e 100644 --- a/platform/commonUI/browse/test/navigation/NavigateActionSpec.js +++ b/platform/commonUI/browse/test/navigation/NavigateActionSpec.js @@ -23,145 +23,74 @@ /** * MCTRepresentationSpec. Created by vwoeltje on 11/6/14. */ -define( - ["../../src/navigation/NavigateAction"], - function (NavigateAction) { +define([ + "../../src/navigation/NavigateAction" +], function ( + NavigateAction +) { - describe("The navigate action", function () { - var mockNavigationService, - mockQ, - mockDomainObject, - mockPolicyService, - mockNavigatedObject, - mockWindow, - capabilities, - action; + describe("The navigate action", function () { + var mockNavigationService, + mockDomainObject, + action; - function mockPromise(value) { - return { - then: function (callback) { - return mockPromise(callback(value)); - } - }; - } - beforeEach(function () { - capabilities = {}; - - mockQ = { when: mockPromise }; - mockNavigatedObject = jasmine.createSpyObj( - "domainObject", - [ - "getId", - "getModel", - "hasCapability", - "getCapability" - ] - ); - - capabilities.editor = jasmine.createSpyObj("editorCapability", [ - "isEditContextRoot", - "finish" - ]); - - mockNavigatedObject.getCapability.andCallFake(function (capability) { - return capabilities[capability]; - }); - mockNavigatedObject.hasCapability.andReturn(false); - - mockNavigationService = jasmine.createSpyObj( - "navigationService", - [ - "setNavigation", - "getNavigation" - ] - ); - mockNavigationService.getNavigation.andReturn(mockNavigatedObject); - - mockDomainObject = jasmine.createSpyObj( - "domainObject", - [ - "getId", - "getModel" - ] - ); - - mockPolicyService = jasmine.createSpyObj("policyService", - [ - "allow" - ]); - mockWindow = jasmine.createSpyObj("$window", - [ - "confirm" - ]); - - action = new NavigateAction( - mockNavigationService, - mockQ, - mockPolicyService, - mockWindow, - { domainObject: mockDomainObject } - ); + function waitForCall() { + var called = false; + waitsFor(function () { + return called; }); + return function () { + called = true; + }; + } - it("invokes the policy service to determine if navigation" + - " allowed", function () { - action.perform(); - expect(mockPolicyService.allow) - .toHaveBeenCalledWith("navigation", jasmine.any(Object), jasmine.any(Object), jasmine.any(Function)); - }); + beforeEach(function () { + mockNavigationService = jasmine.createSpyObj( + "navigationService", + [ + "shouldNavigate", + "setNavigation" + ] + ); - it("prompts user if policy rejection", function () { - action.perform(); - expect(mockPolicyService.allow).toHaveBeenCalled(); - mockPolicyService.allow.mostRecentCall.args[3](); - expect(mockWindow.confirm).toHaveBeenCalled(); - }); - - describe("shows a prompt", function () { - beforeEach(function () { - // Ensure the allow callback is called synchronously - mockPolicyService.allow.andCallFake(function () { - return arguments[3](); - }); - }); - it("does not navigate on prompt rejection", function () { - mockWindow.confirm.andReturn(false); - action.perform(); - expect(mockNavigationService.setNavigation) - .not.toHaveBeenCalled(); - }); - - it("does navigate on prompt acceptance", function () { - mockWindow.confirm.andReturn(true); - action.perform(); - expect(mockNavigationService.setNavigation) - .toHaveBeenCalled(); - }); - }); - - describe("in edit mode", function () { - beforeEach(function () { - mockNavigatedObject.hasCapability.andCallFake(function (capability) { - return capability === "editor"; - }); - capabilities.editor.isEditContextRoot.andReturn(true); - }); - - it("finishes editing if in edit mode", function () { - action.perform(); - expect(capabilities.editor.finish) - .toHaveBeenCalled(); - }); - }); - - it("is only applicable when a domain object is in context", function () { - expect(NavigateAction.appliesTo({})).toBeFalsy(); - expect(NavigateAction.appliesTo({ - domainObject: mockDomainObject - })).toBeTruthy(); - }); + mockDomainObject = {}; + action = new NavigateAction( + mockNavigationService, + { domainObject: mockDomainObject } + ); }); - } -); + + it("sets navigation if it is allowed", function () { + mockNavigationService.shouldNavigate.andReturn(true); + action.perform() + .then(waitForCall()); + runs(function () { + expect(mockNavigationService.setNavigation) + .toHaveBeenCalledWith(mockDomainObject, true); + }); + }); + + it("does not set navigation if it is not allowed", function () { + mockNavigationService.shouldNavigate.andReturn(false); + var onSuccess = jasmine.createSpy('onSuccess'); + action.perform() + .then(onSuccess, waitForCall()); + runs(function () { + expect(onSuccess).not.toHaveBeenCalled(); + expect(mockNavigationService.setNavigation) + .not + .toHaveBeenCalledWith(mockDomainObject); + }); + }); + + it("is only applicable when a domain object is in context", function () { + expect(NavigateAction.appliesTo({})).toBeFalsy(); + expect(NavigateAction.appliesTo({ + domainObject: mockDomainObject + })).toBeTruthy(); + }); + + }); +}); diff --git a/platform/commonUI/browse/test/navigation/NavigationServiceSpec.js b/platform/commonUI/browse/test/navigation/NavigationServiceSpec.js index 8f03a3f090..5a2cb63f0b 100644 --- a/platform/commonUI/browse/test/navigation/NavigationServiceSpec.js +++ b/platform/commonUI/browse/test/navigation/NavigationServiceSpec.js @@ -28,10 +28,12 @@ define( function (NavigationService) { describe("The navigation service", function () { - var navigationService; + var $window, + navigationService; beforeEach(function () { - navigationService = new NavigationService(); + $window = jasmine.createSpyObj('$window', ['confirm']); + navigationService = new NavigationService($window); }); it("stores navigation state", function () { diff --git a/platform/commonUI/edit/src/controllers/EditObjectController.js b/platform/commonUI/edit/src/controllers/EditObjectController.js index e2d4b5e60a..86438b1ae9 100644 --- a/platform/commonUI/edit/src/controllers/EditObjectController.js +++ b/platform/commonUI/edit/src/controllers/EditObjectController.js @@ -70,7 +70,7 @@ define( cancelEditing(domainObject); }); - function setViewForDomainObject(domainObject) { + function setViewForDomainObject() { var locationViewKey = $location.search().view; @@ -87,7 +87,7 @@ define( } } - setViewForDomainObject($scope.domainObject); + setViewForDomainObject(); $scope.doAction = function (action) { return $scope[action] && $scope[action](); diff --git a/platform/commonUI/edit/test/controllers/EditControllerSpec.js b/platform/commonUI/edit/test/controllers/EditObjectControllerSpec.js similarity index 61% rename from platform/commonUI/edit/test/controllers/EditControllerSpec.js rename to platform/commonUI/edit/test/controllers/EditObjectControllerSpec.js index c02d694b08..60c3d9a8ee 100644 --- a/platform/commonUI/edit/test/controllers/EditControllerSpec.js +++ b/platform/commonUI/edit/test/controllers/EditObjectControllerSpec.js @@ -24,32 +24,19 @@ define( ["../../src/controllers/EditObjectController"], function (EditObjectController) { - describe("The Edit mode controller", function () { + describe("The Edit Object controller", function () { var mockScope, mockObject, - mockType, + testViews, + mockEditorCapability, mockLocation, + mockNavigationService, + removeCheck, mockStatusCapability, mockCapabilities, - mockPolicyService, controller; - // Utility function; look for a $watch on scope and fire it - function fireWatch(expr, value) { - mockScope.$watch.calls.forEach(function (call) { - if (call.args[0] === expr) { - call.args[1](value); - } - }); - } - beforeEach(function () { - mockPolicyService = jasmine.createSpyObj( - "policyService", - [ - "allow" - ] - ); mockScope = jasmine.createSpyObj( "$scope", ["$on", "$watch"] @@ -58,16 +45,16 @@ define( "domainObject", ["getId", "getModel", "getCapability", "hasCapability", "useCapability"] ); - mockType = jasmine.createSpyObj( - "type", - ["hasFeature"] + mockEditorCapability = jasmine.createSpyObj( + "mockEditorCapability", + ["isEditContextRoot", "dirty", "finish"] ); mockStatusCapability = jasmine.createSpyObj('statusCapability', ["get"] ); mockCapabilities = { - "type" : mockType, + "editor" : mockEditorCapability, "status": mockStatusCapability }; @@ -75,52 +62,70 @@ define( ["search"] ); mockLocation.search.andReturn({"view": "fixed"}); + mockNavigationService = jasmine.createSpyObj('navigationService', + ["checkBeforeNavigation"] + ); + + removeCheck = jasmine.createSpy('removeCheck'); + mockNavigationService.checkBeforeNavigation.andReturn(removeCheck); mockObject.getId.andReturn("test"); mockObject.getModel.andReturn({ name: "Test object" }); mockObject.getCapability.andCallFake(function (key) { return mockCapabilities[key]; }); - mockType.hasFeature.andReturn(true); - mockScope.domainObject = mockObject; - - controller = new EditObjectController( - mockScope, - mockLocation, - mockPolicyService - ); - }); - - it("exposes a warning message for unload", function () { - var errorMessage = "Unsaved changes"; - - // Normally, should be undefined - expect(controller.getUnloadWarning()).toBeUndefined(); - - // Override the policy service to prevent navigation - mockPolicyService.allow.andCallFake(function (category, object, context, callback) { - callback(errorMessage); - }); - - // Should have some warning message here now - expect(controller.getUnloadWarning()).toEqual(errorMessage); - }); - - - it("sets the active view from query parameters", function () { - var testViews = [ - { key: 'abc' }, - { key: 'def', someKey: 'some value' }, - { key: 'xyz' } - ]; + testViews = [ + { key: 'abc' }, + { key: 'def', someKey: 'some value' }, + { key: 'xyz' } + ]; mockObject.useCapability.andCallFake(function (c) { return (c === 'view') && testViews; }); mockLocation.search.andReturn({ view: 'def' }); - fireWatch('domainObject', mockObject); + mockScope.domainObject = mockObject; + + controller = new EditObjectController( + mockScope, + mockLocation, + mockNavigationService + ); + }); + + it("adds a check before navigation", function () { + expect(mockNavigationService.checkBeforeNavigation) + .toHaveBeenCalledWith(jasmine.any(Function)); + + var checkFn = mockNavigationService.checkBeforeNavigation.mostRecentCall.args[0]; + + mockEditorCapability.isEditContextRoot.andReturn(false); + mockEditorCapability.dirty.andReturn(false); + + expect(checkFn()).toBe(false); + + mockEditorCapability.isEditContextRoot.andReturn(true); + expect(checkFn()).toBe(false); + + mockEditorCapability.dirty.andReturn(true); + expect(checkFn()) + .toBe("Continuing will cause the loss of any unsaved changes."); + + }); + + it("cleans up on destroy", function () { + expect(mockScope.$on) + .toHaveBeenCalledWith("$destroy", jasmine.any(Function)); + + mockScope.$on.mostRecentCall.args[1](); + + expect(mockEditorCapability.finish).toHaveBeenCalled(); + expect(removeCheck).toHaveBeenCalled(); + }); + + it("sets the active view from query parameters", function () { expect(mockScope.representation.selected) .toEqual(testViews[1]); }); diff --git a/platform/commonUI/edit/test/representers/EditRepresenterSpec.js b/platform/commonUI/edit/test/representers/EditRepresenterSpec.js deleted file mode 100644 index cb766f95a7..0000000000 --- a/platform/commonUI/edit/test/representers/EditRepresenterSpec.js +++ /dev/null @@ -1,122 +0,0 @@ -/***************************************************************************** - * Open MCT, Copyright (c) 2014-2016, United States Government - * as represented by the Administrator of the National Aeronautics and Space - * Administration. All rights reserved. - * - * Open MCT is licensed under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0. - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - * - * Open MCT includes source code licensed under additional open source - * licenses. See the Open Source Licenses file (LICENSES.md) included with - * this source code distribution or the Licensing information page available - * at runtime from the About dialog for additional information. - *****************************************************************************/ - -define( - ["../../src/representers/EditRepresenter"], - function (EditRepresenter) { - - describe("The Edit mode representer", function () { - var mockQ, - mockLog, - mockScope, - testRepresentation, - mockDomainObject, - mockStatusCapability, - mockEditorCapability, - mockCapabilities, - representer; - - function mockPromise(value) { - return { - then: function (callback) { - return mockPromise(callback(value)); - } - }; - } - - beforeEach(function () { - mockQ = { when: mockPromise }; - mockLog = jasmine.createSpyObj("$log", ["info", "debug"]); - mockScope = jasmine.createSpyObj("$scope", ["$watch", "$on"]); - testRepresentation = { key: "test" }; - mockDomainObject = jasmine.createSpyObj("domainObject", [ - "getId", - "getModel", - "getCapability", - "useCapability", - "hasCapability" - ]); - mockStatusCapability = - jasmine.createSpyObj("statusCapability", ["listen"]); - mockEditorCapability = - jasmine.createSpyObj("editorCapability", ["isEditContextRoot"]); - - mockCapabilities = { - 'status': mockStatusCapability, - 'editor': mockEditorCapability - }; - - mockDomainObject.getModel.andReturn({}); - mockDomainObject.hasCapability.andReturn(true); - mockDomainObject.useCapability.andReturn(true); - mockDomainObject.getCapability.andCallFake(function (capability) { - return mockCapabilities[capability]; - }); - - representer = new EditRepresenter(mockQ, mockLog, mockScope); - representer.represent(testRepresentation, mockDomainObject); - }); - - it("provides a commit method in scope", function () { - expect(mockScope.commit).toEqual(jasmine.any(Function)); - }); - - it("Sets edit view template on edit mode", function () { - mockStatusCapability.listen.mostRecentCall.args[0](['editing']); - mockEditorCapability.isEditContextRoot.andReturn(true); - expect(mockScope.viewObjectTemplate).toEqual('edit-object'); - }); - - it("Cleans up listeners on scope destroy", function () { - representer.listenHandle = jasmine.createSpy('listen'); - mockScope.$on.mostRecentCall.args[1](); - expect(representer.listenHandle).toHaveBeenCalled(); - }); - - it("mutates upon observed changes", function () { - mockScope.model = { someKey: "some value" }; - mockScope.configuration = { someConfiguration: "something" }; - - mockScope.commit("Some message"); - - // Should have mutated the object... - expect(mockDomainObject.useCapability).toHaveBeenCalledWith( - "mutation", - jasmine.any(Function) - ); - - // Finally, check that the provided mutation function - // includes both model and configuration - expect( - mockDomainObject.useCapability.mostRecentCall.args[1]() - ).toEqual({ - someKey: "some value", - configuration: { - test: { someConfiguration: "something" } - } - }); - }); - - - }); - } -); diff --git a/platform/commonUI/general/src/directives/MCTTree.js b/platform/commonUI/general/src/directives/MCTTree.js index cae72d497b..d5ab64fc30 100644 --- a/platform/commonUI/general/src/directives/MCTTree.js +++ b/platform/commonUI/general/src/directives/MCTTree.js @@ -29,7 +29,7 @@ define([ if (!scope.allowSelection) { scope.allowSelection = function () { return true; - } + }; } if (!scope.onSelection) { scope.onSelection = function () {}; diff --git a/platform/commonUI/general/test/directives/MCTTreeSpec.js b/platform/commonUI/general/test/directives/MCTTreeSpec.js index bd8e17117a..3512731429 100644 --- a/platform/commonUI/general/test/directives/MCTTreeSpec.js +++ b/platform/commonUI/general/test/directives/MCTTreeSpec.js @@ -19,10 +19,12 @@ * this source code distribution or the Licensing information page available * at runtime from the About dialog for additional information. *****************************************************************************/ +/* global console*/ define([ - '../../src/directives/MCTTree' -], function (MCTTree) { + '../../src/directives/MCTTree', + '../../src/ui/TreeView' +], function (MCTTree, TreeView) { describe("The mct-tree directive", function () { var mockParse, mockGestureService, @@ -50,6 +52,7 @@ define([ mockExpr = jasmine.createSpy('expr'); mockExpr.assign = jasmine.createSpy('assign'); mockParse.andReturn(mockExpr); + spyOn(TreeView.prototype, 'observe').andCallThrough(); mctTree = new MCTTree(mockParse, mockGestureService); }); @@ -58,8 +61,13 @@ define([ expect(mctTree.restrict).toEqual("E"); }); - it("two-way binds to mctObject and mctModel", function () { - expect(mctTree.scope).toEqual({ mctObject: "=", mctModel: "=" }); + it("two-way binds", function () { + expect(mctTree.scope).toEqual({ + rootObject: "=", + selectedObject: "=", + allowSelection: "=?", + onSelection: "=?" + }); }); describe("link", function () { @@ -81,16 +89,16 @@ define([ expect(mockElement.append).toHaveBeenCalled(); }); - it("watches for mct-model's expression in the parent", function () { + it("watches for selected-object expression in the parent", function () { expect(mockScope.$watch).toHaveBeenCalledWith( - "mctModel", + "selectedObject", jasmine.any(Function) ); }); - it("watches for changes to mct-object", function () { + it("watches for changes to root-object", function () { expect(mockScope.$watch).toHaveBeenCalledWith( - "mctObject", + "rootObject", jasmine.any(Function) ); }); @@ -102,6 +110,10 @@ define([ ); }); + it("watches for changes in tree view", function () { + + }); + // https://github.com/nasa/openmct/issues/1114 it("does not trigger $apply during $watches", function () { mockScope.mctObject = makeMockDomainObject('root'); @@ -111,14 +123,18 @@ define([ }); expect(mockScope.$apply).not.toHaveBeenCalled(); }); - it("does trigger $apply from other value changes", function () { + it("does trigger $apply from tree manipulation", function () { + if (/PhantomJS/g.test(window.navigator.userAgent)) { + console.log('Unable to run test in PhantomJS due to lack of support for event constructors'); + return; + } // White-boxy; we know this is the setter for the tree's value - var treeValueFn = mockScope.$watch.calls[0].args[1]; + var treeValueFn = TreeView.prototype.observe.calls[0].args[0]; mockScope.mctObject = makeMockDomainObject('root'); mockScope.mctMode = makeMockDomainObject('selection'); - treeValueFn(makeMockDomainObject('other')); + treeValueFn(makeMockDomainObject('other'), new MouseEvent("click")); expect(mockScope.$apply).toHaveBeenCalled(); }); diff --git a/platform/commonUI/general/test/ui/TreeViewSpec.js b/platform/commonUI/general/test/ui/TreeViewSpec.js index 4337bc5bcd..ef8514a60b 100644 --- a/platform/commonUI/general/test/ui/TreeViewSpec.js +++ b/platform/commonUI/general/test/ui/TreeViewSpec.js @@ -289,8 +289,9 @@ define([ }); it("notifies listeners when value is changed", function () { - treeView.value(mockDomainObject); - expect(mockCallback).toHaveBeenCalledWith(mockDomainObject); + treeView.value(mockDomainObject, {some: event}); + expect(mockCallback) + .toHaveBeenCalledWith(mockDomainObject, {some: event}); }); it("does not notify listeners when deactivated", function () {