From 757cb0f0157be7ea29db1e53da33272516ea4578 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 10:18:59 -0700 Subject: [PATCH 01/26] [Representation] Watch for key changes ...from mct-include. Improves behavior of that directive and supports testing of changes to remove whole elements when inapplicable, nasa/openmctweb#195. --- platform/representation/src/MCTInclude.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index dc00c8b89d..768a84a5bc 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -70,8 +70,10 @@ define( }); function controller($scope) { - // Pass the template URL to ng-include via scope. - $scope.inclusion = templateMap[$scope.key]; + $scope.$watch('key', function (key) { + // Pass the template URL to ng-include via scope. + $scope.inclusion = templateMap[$scope.key]; + }); } return { From 942f617bd8b2a6d8bd31c88a1b1827142e8f39da Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 10:29:41 -0700 Subject: [PATCH 02/26] [Representation] Use transclusion for mct-include ...to add/remove conditionally depending on the existence of certain templates. Note that this currently breaks mct-include due to an incompatibility between element transclusion and directive templates; see https://github.com/angular/angular.js/issues/3368. --- platform/representation/src/MCTInclude.js | 28 +++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index 768a84a5bc..88d3a3378a 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -69,19 +69,39 @@ define( templateMap[key] = templateMap[key] || path; }); - function controller($scope) { + function link($scope, element, attrs, ctrl, transclude) { + var originalElement = element, + activeElement = element; + $scope.$watch('key', function (key) { - // Pass the template URL to ng-include via scope. - $scope.inclusion = templateMap[$scope.key]; + if (templateMap[key]) { + // Pass the template URL to ng-include via scope. + $scope.inclusion = templateMap[$scope.key]; + // ...and add the template to the DOM. + transclude(function (clone) { + activeElement.replaceWith(clone); + activeElement = clone; + }); + } else if (activeElement !== originalElement) { + // If the key is unknown, remove it from DOM entirely. + activeElement.replaceWith(originalElement); + activeElement = originalElement; + } }); } return { + transclude: 'element', + + priority: 601, + + terminal: true, + // Only show at the element level restrict: "E", // Use the included controller to populate scope - controller: controller, + link: link, // Use ng-include as a template; "inclusion" will be the real // template path From b4a44dee8f79e47e5d361206ea3f123c1f4bab50 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 11:13:53 -0700 Subject: [PATCH 03/26] [Representation] Populate mct-include contents Remove usage of ng-include and template from mct-include for compatibility with element-level transclusion. Has useful side effect of pre-fetching templates and reducing watch count. --- platform/representation/bundle.json | 2 +- platform/representation/src/MCTInclude.js | 69 ++++++++++++++--------- 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/platform/representation/bundle.json b/platform/representation/bundle.json index 331856a9a8..6d782b5d95 100644 --- a/platform/representation/bundle.json +++ b/platform/representation/bundle.json @@ -4,7 +4,7 @@ { "key": "mctInclude", "implementation": "MCTInclude.js", - "depends": [ "templates[]", "$sce" ] + "depends": [ "templates[]", "$sce", "$http", "$compile" ] }, { "key": "mctRepresentation", diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index 88d3a3378a..2abbd8ba9c 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -54,9 +54,49 @@ define( * @param {TemplateDefinition[]} templates an array of * template extensions */ - function MCTInclude(templates, $sce) { + function MCTInclude(templates, $sce, $http, $compile, $log) { var templateMap = {}; + function loadTemplate(path) { + return $http.get(path).then(function (response) { + return $compile(response.data); + }); + } + + function addTemplate(key, $scope, element) { + // Pass the template URL to ng-include via scope. + //$scope.inclusion = templateMap[$scope.key]; + // ...and add the template to the DOM. + templateMap[key].then(function (template) { + template($scope, function (innerClone) { + element.append(innerClone); + }); + }, function () { + $log.warn("Could not load template " + key); + delete templateMap[key]; + }); + } + + function link($scope, element, attrs, ctrl, transclude) { + var originalElement = element, + activeElement = element; + + $scope.$watch('key', function (key) { + if (templateMap[key]) { + transclude(function (clone) { + activeElement.replaceWith(clone); + activeElement = clone; + activeElement.empty(); + addTemplate(key, $scope, activeElement); + }); + } else if (activeElement !== originalElement) { + // If the key is unknown, remove it from DOM entirely. + activeElement.replaceWith(originalElement); + activeElement = originalElement; + } + }); + } + // Prepopulate templateMap for easy look up by key templates.forEach(function (template) { var key = template.key, @@ -66,30 +106,9 @@ define( template.templateUrl ].join("/")); // First found should win (priority ordering) - templateMap[key] = templateMap[key] || path; + templateMap[key] = templateMap[key] || loadTemplate(path); }); - function link($scope, element, attrs, ctrl, transclude) { - var originalElement = element, - activeElement = element; - - $scope.$watch('key', function (key) { - if (templateMap[key]) { - // Pass the template URL to ng-include via scope. - $scope.inclusion = templateMap[$scope.key]; - // ...and add the template to the DOM. - transclude(function (clone) { - activeElement.replaceWith(clone); - activeElement = clone; - }); - } else if (activeElement !== originalElement) { - // If the key is unknown, remove it from DOM entirely. - activeElement.replaceWith(originalElement); - activeElement = originalElement; - } - }); - } - return { transclude: 'element', @@ -103,10 +122,6 @@ define( // Use the included controller to populate scope link: link, - // Use ng-include as a template; "inclusion" will be the real - // template path - template: '', - // Two-way bind key, ngModel, and parameters scope: { key: "=", ngModel: "=", parameters: "=" } }; From 3d59f6df0bc0a514845dbcce0be1f11be474dedc Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 14:51:30 -0700 Subject: [PATCH 04/26] [Representation] Separate out template loading/linking ...from mct-include, to facilitate reuse for MCTRepresentation. --- platform/representation/bundle.json | 8 +- platform/representation/src/MCTInclude.js | 52 ++------- platform/representation/src/TemplateLinker.js | 106 ++++++++++++++++++ 3 files changed, 125 insertions(+), 41 deletions(-) create mode 100644 platform/representation/src/TemplateLinker.js diff --git a/platform/representation/bundle.json b/platform/representation/bundle.json index 6d782b5d95..51ad8a0af8 100644 --- a/platform/representation/bundle.json +++ b/platform/representation/bundle.json @@ -4,7 +4,7 @@ { "key": "mctInclude", "implementation": "MCTInclude.js", - "depends": [ "templates[]", "$sce", "$http", "$compile" ] + "depends": [ "templates[]", "templateLinker" ] }, { "key": "mctRepresentation", @@ -48,6 +48,12 @@ "key": "dndService", "implementation": "services/DndService.js", "depends": [ "$log" ] + }, + { + "key": "templateLinker", + "implementation": "TemplateLinker.js", + "depends": [ "$http", "$compile", "$log" ], + "comment": "For internal use by mct-include and mct-representation." } ], "actions": [ diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index 2abbd8ba9c..400a9d3ad8 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -54,59 +54,31 @@ define( * @param {TemplateDefinition[]} templates an array of * template extensions */ - function MCTInclude(templates, $sce, $http, $compile, $log) { + function MCTInclude(templates, templateLinker) { var templateMap = {}; - function loadTemplate(path) { - return $http.get(path).then(function (response) { - return $compile(response.data); - }); - } + function link(scope, element, attrs, ctrl, transclude) { + var changeTemplates = templateLinker.link( + scope, + element, + transclude + ); - function addTemplate(key, $scope, element) { - // Pass the template URL to ng-include via scope. - //$scope.inclusion = templateMap[$scope.key]; - // ...and add the template to the DOM. - templateMap[key].then(function (template) { - template($scope, function (innerClone) { - element.append(innerClone); - }); - }, function () { - $log.warn("Could not load template " + key); - delete templateMap[key]; - }); - } - - function link($scope, element, attrs, ctrl, transclude) { - var originalElement = element, - activeElement = element; - - $scope.$watch('key', function (key) { - if (templateMap[key]) { - transclude(function (clone) { - activeElement.replaceWith(clone); - activeElement = clone; - activeElement.empty(); - addTemplate(key, $scope, activeElement); - }); - } else if (activeElement !== originalElement) { - // If the key is unknown, remove it from DOM entirely. - activeElement.replaceWith(originalElement); - activeElement = originalElement; - } + scope.$watch('key', function (key) { + changeTemplates(templateMap[key]); }); } // Prepopulate templateMap for easy look up by key templates.forEach(function (template) { var key = template.key, - path = $sce.trustAsResourceUrl([ + path = [ template.bundle.path, template.bundle.resources, template.templateUrl - ].join("/")); + ].join("/"); // First found should win (priority ordering) - templateMap[key] = templateMap[key] || loadTemplate(path); + templateMap[key] = templateMap[key] || path; }); return { diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js new file mode 100644 index 0000000000..0dfa180d51 --- /dev/null +++ b/platform/representation/src/TemplateLinker.js @@ -0,0 +1,106 @@ +/***************************************************************************** + * Open MCT Web, Copyright (c) 2014-2015, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT Web 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 Web 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. + *****************************************************************************/ +/*global define,Promise*/ + +define( + [], + function () { + "use strict"; + + /** + * Pre-fetches templates and allows them to be + * + * @param {string[]} URLs to all templates which should be loadable + * @param $http Angular's `$http` service + * @param {Function} $compile Angular's `$compile` service + * @param $log Angular's `$log` service + */ + function TemplateLinker($http, $compile, $log) { + this.templateMap = {}; + this.$http = $http; + this.$compile = $compile; + this.$log = $log; + } + + TemplateLinker.prototype.load = function (templateUrl) { + var $http = this.$http, + $compile = this.$compile, + $log = this.$log, + templateMap = this.templateMap; + + return (templateMap[templateUrl] = templateMap[templateUrl] || + $http.get(templateUrl).then(function (response) { + return $compile(response.data); + }, function () { + $log.warn("Couldn't load template at " + templateUrl); + templateMap[templateUrl] = undefined; + })); + }; + + /** + * @returns {Function} a function which can be called with a template + * URL to switch templates, or `undefined` to remove. + */ + TemplateLinker.prototype.link = function (scope, element, transclude) { + var originalElement = element, + activeElement = element, + self = this; + + function removeElement() { + if (activeElement !== originalElement) { + activeElement.replaceWith(originalElement); + activeElement = originalElement; + } + } + + function replaceElement(clone, template) { + activeElement.replaceWith(clone); + activeElement = clone; + activeElement.empty(); + template(scope, function (innerClone) { + clone.append(innerClone); + }); + } + + function applyTemplate(template) { + if (template) { + transclude(function (clone) { + replaceElement(clone, template); + }); + } else { + removeElement(); + } + } + + return function (templateUrl) { + if (templateUrl) { + self.load(templateUrl).then(applyTemplate); + } else { + removeElement(); + } + }; + }; + + return TemplateLinker; + } +); + From ab008ae49765b5e8a5a010ee65170b00fb5a0d61 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 15:17:36 -0700 Subject: [PATCH 05/26] [Representation] Begin integration ...of templateLinker into mct-representation. Not working currently due to prevalence of mct-representation instances with transcluding directives (hitting a multiple transclusion error.) --- platform/representation/bundle.json | 2 +- .../representation/src/MCTRepresentation.js | 27 ++++++++++++------- platform/representation/src/TemplateLinker.js | 18 ++++++++----- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/platform/representation/bundle.json b/platform/representation/bundle.json index 51ad8a0af8..7d6ddd3113 100644 --- a/platform/representation/bundle.json +++ b/platform/representation/bundle.json @@ -9,7 +9,7 @@ { "key": "mctRepresentation", "implementation": "MCTRepresentation.js", - "depends": [ "representations[]", "views[]", "representers[]", "$q", "$sce", "$log" ] + "depends": [ "representations[]", "views[]", "representers[]", "$q", "templateLinker", "$log" ] } ], "gestures": [ diff --git a/platform/representation/src/MCTRepresentation.js b/platform/representation/src/MCTRepresentation.js index 98a814c362..cefda36b42 100644 --- a/platform/representation/src/MCTRepresentation.js +++ b/platform/representation/src/MCTRepresentation.js @@ -55,7 +55,7 @@ define( * representation extensions * @param {ViewDefinition[]} views an array of view extensions */ - function MCTRepresentation(representations, views, representers, $q, $sce, $log) { + function MCTRepresentation(representations, views, representers, $q, templateLinker, $log) { var representationMap = {}, gestureMap = {}; @@ -72,11 +72,11 @@ define( // Get a path to a representation function getPath(representation) { - return $sce.trustAsResourceUrl([ + return [ representation.bundle.path, representation.bundle.resources, representation.templateUrl - ].join("/")); + ].join("/"); } // Look up a matching representation for this domain object @@ -94,12 +94,17 @@ define( } } - function link($scope, element, attrs) { + function link($scope, element, attrs, ctrl, transclude) { var activeRepresenters = representers.map(function (Representer) { return new Representer($scope, element, attrs); }), toClear = [], // Properties to clear out of scope on change - counter = 0; + counter = 0, + changeTemplate = templateLinker.link( + $scope, + element, + transclude + ); // Populate scope with any capabilities indicated by the // representation's extension definition @@ -158,7 +163,7 @@ define( // Look up the actual template path, pass it to ng-include // via the "inclusion" field - $scope.inclusion = representation && getPath(representation); + changeTemplate(representation && getPath(representation)); // Any existing representers are no longer valid; release them. destroyRepresenters(); @@ -227,16 +232,18 @@ define( } return { + transclude: 'element', + + priority: 601, + + terminal: true, + // Only applicable at the element level restrict: "E", // Handle Angular's linking step link: link, - // Use ng-include as a template; "inclusion" will be the real - // template path - template: '', - // Two-way bind key and parameters, get the represented domain // object as "mct-object" scope: { diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index 0dfa180d51..c3f8eb0208 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -63,6 +63,7 @@ define( TemplateLinker.prototype.link = function (scope, element, transclude) { var originalElement = element, activeElement = element, + activeTemplateUrl, self = this; function removeElement() { @@ -91,13 +92,18 @@ define( } } - return function (templateUrl) { - if (templateUrl) { - self.load(templateUrl).then(applyTemplate); - } else { - removeElement(); + function changeTemplate(templateUrl) { + if (templateUrl !== activeTemplateUrl) { + if (templateUrl) { + self.load(templateUrl).then(applyTemplate); + } else { + removeElement(); + } + activeTemplateUrl = templateUrl; } - }; + } + + return changeTemplate; }; return TemplateLinker; From bcc42d705ea5024f155617b5bceea20d6b6b4fd8 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 15:29:57 -0700 Subject: [PATCH 06/26] [Representation] Hide elements without transclusion --- platform/representation/src/MCTInclude.js | 16 ++------- .../representation/src/MCTRepresentation.js | 34 +++++++------------ platform/representation/src/TemplateLinker.js | 26 +++++++------- 3 files changed, 29 insertions(+), 47 deletions(-) diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index 400a9d3ad8..e00ef8599d 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -57,15 +57,11 @@ define( function MCTInclude(templates, templateLinker) { var templateMap = {}; - function link(scope, element, attrs, ctrl, transclude) { - var changeTemplates = templateLinker.link( - scope, - element, - transclude - ); + function link(scope, element) { + var changeTemplate = templateLinker.link(scope, element); scope.$watch('key', function (key) { - changeTemplates(templateMap[key]); + changeTemplate(templateMap[key]); }); } @@ -82,12 +78,6 @@ define( }); return { - transclude: 'element', - - priority: 601, - - terminal: true, - // Only show at the element level restrict: "E", diff --git a/platform/representation/src/MCTRepresentation.js b/platform/representation/src/MCTRepresentation.js index cefda36b42..9d5f612960 100644 --- a/platform/representation/src/MCTRepresentation.js +++ b/platform/representation/src/MCTRepresentation.js @@ -100,11 +100,7 @@ define( }), toClear = [], // Properties to clear out of scope on change counter = 0, - changeTemplate = templateLinker.link( - $scope, - element, - transclude - ); + changeTemplate = templateLinker.link($scope, element); // Populate scope with any capabilities indicated by the // representation's extension definition @@ -155,15 +151,9 @@ define( function refresh() { var domainObject = $scope.domainObject, representation = lookup($scope.key, domainObject), - uses = ((representation || {}).uses || []); - - // Create an empty object named "representation", for this - // representation to store local variables into. - $scope.representation = {}; - - // Look up the actual template path, pass it to ng-include - // via the "inclusion" field - changeTemplate(representation && getPath(representation)); + path = representation && getPath(representation), + uses = ((representation || {}).uses || []), + canRepresent = !!(path && domainObject); // Any existing representers are no longer valid; release them. destroyRepresenters(); @@ -179,9 +169,17 @@ define( delete $scope[property]; }); + // Create an empty object named "representation", for this + // representation to store local variables into. + $scope.representation = {}; + + // Change templates (passing in undefined to clear + // if we don't have enough info to show a template.) + changeTemplate(canRepresent ? path : undefined); + // Populate scope with fields associated with the current // domain object (if one has been passed in) - if (domainObject) { + if (canRepresent) { // Track how many representations we've made in this scope, // to ensure that the correct representations are matched to // the correct object/key pairs. @@ -232,12 +230,6 @@ define( } return { - transclude: 'element', - - priority: 601, - - terminal: true, - // Only applicable at the element level restrict: "E", diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index c3f8eb0208..0c5e76cca7 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -60,33 +60,31 @@ define( * @returns {Function} a function which can be called with a template * URL to switch templates, or `undefined` to remove. */ - TemplateLinker.prototype.link = function (scope, element, transclude) { - var originalElement = element, - activeElement = element, + TemplateLinker.prototype.link = function (scope, element) { + var activeElement = element, activeTemplateUrl, + comment = this.$compile('')(scope), self = this; function removeElement() { - if (activeElement !== originalElement) { - activeElement.replaceWith(originalElement); - activeElement = originalElement; + if (activeElement !== comment) { + activeElement.replaceWith(comment); + activeElement = comment; } } - function replaceElement(clone, template) { - activeElement.replaceWith(clone); - activeElement = clone; + function replaceElement(template) { + activeElement.replaceWith(element); + activeElement = element; activeElement.empty(); template(scope, function (innerClone) { - clone.append(innerClone); + element.append(innerClone); }); } function applyTemplate(template) { if (template) { - transclude(function (clone) { - replaceElement(clone, template); - }); + replaceElement(template); } else { removeElement(); } @@ -103,6 +101,8 @@ define( } } + removeElement(); + return changeTemplate; }; From 567754829848e8682d22db9d82f80dbd43783950 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 15:32:50 -0700 Subject: [PATCH 07/26] [Representation] Show element synchronously ...to avoid exceptions when trying to invoke representers before an mct-representation has been added back into the DOM. --- platform/representation/src/TemplateLinker.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index 0c5e76cca7..fd5ba57a3c 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -73,10 +73,15 @@ define( } } - function replaceElement(template) { - activeElement.replaceWith(element); - activeElement = element; - activeElement.empty(); + function addElement() { + if (activeElement !== element) { + activeElement.replaceWith(element); + activeElement = element; + activeElement.empty(); + } + } + + function populateElement(template) { template(scope, function (innerClone) { element.append(innerClone); }); @@ -84,7 +89,7 @@ define( function applyTemplate(template) { if (template) { - replaceElement(template); + populateElement(template); } else { removeElement(); } @@ -93,6 +98,7 @@ define( function changeTemplate(templateUrl) { if (templateUrl !== activeTemplateUrl) { if (templateUrl) { + addElement(); self.load(templateUrl).then(applyTemplate); } else { removeElement(); From 04043030422849dfea809aaf58f38d0f8f3cb826 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 15:38:11 -0700 Subject: [PATCH 08/26] [Representation] Move changes back Move changed lines back to their original location to simplify diff. --- platform/representation/src/MCTRepresentation.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/platform/representation/src/MCTRepresentation.js b/platform/representation/src/MCTRepresentation.js index 9d5f612960..3c09edbfc4 100644 --- a/platform/representation/src/MCTRepresentation.js +++ b/platform/representation/src/MCTRepresentation.js @@ -155,6 +155,14 @@ define( uses = ((representation || {}).uses || []), canRepresent = !!(path && domainObject); + // Create an empty object named "representation", for this + // representation to store local variables into. + $scope.representation = {}; + + // Change templates (passing in undefined to clear + // if we don't have enough info to show a template.) + changeTemplate(canRepresent ? path : undefined); + // Any existing representers are no longer valid; release them. destroyRepresenters(); @@ -169,14 +177,6 @@ define( delete $scope[property]; }); - // Create an empty object named "representation", for this - // representation to store local variables into. - $scope.representation = {}; - - // Change templates (passing in undefined to clear - // if we don't have enough info to show a template.) - changeTemplate(canRepresent ? path : undefined); - // Populate scope with fields associated with the current // domain object (if one has been passed in) if (canRepresent) { From ea9f607bba6c0cf390d1f867768ef6f53ef40675 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 15:46:47 -0700 Subject: [PATCH 09/26] [Representation] Handle undefined scope element.scope() may be undefined when wiring in the info gesture, so check for that. That this is sometimes undefined appears to be a consequence of changes to mct-representation, but which changes influence this are unclear. In any event, it appears that this cannot be relied-upon per https://github.com/angular/angular.js/issues/9515 --- .../commonUI/inspect/src/gestures/InfoGesture.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/inspect/src/gestures/InfoGesture.js b/platform/commonUI/inspect/src/gestures/InfoGesture.js index 09740841e4..6be25e646d 100644 --- a/platform/commonUI/inspect/src/gestures/InfoGesture.js +++ b/platform/commonUI/inspect/src/gestures/InfoGesture.js @@ -55,11 +55,6 @@ define( self.trackPosition(event); }; - // Also make sure we dismiss bubble if representation is destroyed - // before the mouse actually leaves it - this.scopeOff = - element.scope().$on('$destroy', this.hideBubbleCallback); - this.element = element; this.$timeout = $timeout; this.infoService = infoService; @@ -131,6 +126,13 @@ define( this.pendingBubble = this.$timeout(displayBubble, this.delay); this.element.on('mouseleave', this.hideBubbleCallback); + + // Also make sure we dismiss bubble if representation is destroyed + // before the mouse actually leaves it + this.scopeOff = + this.element.scope() && + this.element.scope().$on('$destroy', this.hideBubbleCallback); + }; @@ -143,7 +145,9 @@ define( this.hideBubble(); // ...and detach listeners this.element.off('mouseenter', this.showBubbleCallback); - this.scopeOff(); + if (this.scopeOff) { + this.scopeOff(); + } }; return InfoGesture; From c5fcc5a5580436e478a2d28e8b493016e5485120 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 15:57:47 -0700 Subject: [PATCH 10/26] [Representation] Handle edge cases Handle edge cases (e.g. directive priorities, race conditions) to ensure that mct-representation and mct-include display correctly when added to or removed from the DOM. --- platform/representation/src/MCTInclude.js | 3 +++ platform/representation/src/MCTRepresentation.js | 3 +++ platform/representation/src/TemplateLinker.js | 10 ++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index e00ef8599d..d56d2f0e69 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -84,6 +84,9 @@ define( // Use the included controller to populate scope link: link, + // May hide the element, so let other directives act first + priority: -1000, + // Two-way bind key, ngModel, and parameters scope: { key: "=", ngModel: "=", parameters: "=" } }; diff --git a/platform/representation/src/MCTRepresentation.js b/platform/representation/src/MCTRepresentation.js index 3c09edbfc4..4704c0265e 100644 --- a/platform/representation/src/MCTRepresentation.js +++ b/platform/representation/src/MCTRepresentation.js @@ -236,6 +236,9 @@ define( // Handle Angular's linking step link: link, + // May hide the element, so let other directives act first + priority: -1000, + // Two-way bind key and parameters, get the represented domain // object as "mct-object" scope: { diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index fd5ba57a3c..29df511427 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -83,11 +83,12 @@ define( function populateElement(template) { template(scope, function (innerClone) { + element.empty(); element.append(innerClone); }); } - function applyTemplate(template) { + function applyTemplate(template, templateUrl) { if (template) { populateElement(template); } else { @@ -99,7 +100,12 @@ define( if (templateUrl !== activeTemplateUrl) { if (templateUrl) { addElement(); - self.load(templateUrl).then(applyTemplate); + self.load(templateUrl).then(function (template) { + // Avoid race conditions + if (templateUrl === activeTemplateUrl) { + applyTemplate(template); + } + }); } else { removeElement(); } From 5604bf6d69052816338789e84ba25ef1b8ce8d16 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 16:01:55 -0700 Subject: [PATCH 11/26] [Representation] Add template tester ...to simply verification of mct-include/mct-representation behavior. --- example/templates/bundle.json | 17 +++++++++++++++++ example/templates/res/templates.html | 14 ++++++++++++++ .../templates/src/TemplateTestingController.js | 18 ++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 example/templates/bundle.json create mode 100644 example/templates/res/templates.html create mode 100644 example/templates/src/TemplateTestingController.js diff --git a/example/templates/bundle.json b/example/templates/bundle.json new file mode 100644 index 0000000000..38989c408c --- /dev/null +++ b/example/templates/bundle.json @@ -0,0 +1,17 @@ +{ + "extensions": { + "routes": [ + { + "when": "/templates", + "templateUrl": "templates.html" + } + ], + "controllers": [ + { + "implementation": "TemplateTestingController.js", + "key": "TTC", + "depends": [ "$scope", "objectService" ] + } + ] + } +} diff --git a/example/templates/res/templates.html b/example/templates/res/templates.html new file mode 100644 index 0000000000..ac2747acaa --- /dev/null +++ b/example/templates/res/templates.html @@ -0,0 +1,14 @@ +
+
+
mct-include
+ + +
+ +
+
mct-representation
+ + + +
+
diff --git a/example/templates/src/TemplateTestingController.js b/example/templates/src/TemplateTestingController.js new file mode 100644 index 0000000000..ccb4223603 --- /dev/null +++ b/example/templates/src/TemplateTestingController.js @@ -0,0 +1,18 @@ +/*global define*/ +define([], function () { + 'use strict'; + + return function TemplateTestingController($scope, objectService) { + $scope.ikey = ''; + $scope.rkey = ''; + $scope.rid = ''; + $scope.obj = undefined; + + $scope.$watch('rid', function (id) { + $scope.obj = undefined; + objectService.getObjects([id]).then(function (objs) { + $scope.obj = objs[id]; + }); + }); + }; +}); From ae0cb63a92ad610bc8bf0dc5f3afa8429a60999c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 16:02:52 -0700 Subject: [PATCH 12/26] [Representation] Remove template tester Remove template tester after verifying that mct-include/mct-representation elements are added/removed to/from the DOM as appropriate. --- example/templates/bundle.json | 17 ----------------- example/templates/res/templates.html | 14 -------------- .../templates/src/TemplateTestingController.js | 18 ------------------ 3 files changed, 49 deletions(-) delete mode 100644 example/templates/bundle.json delete mode 100644 example/templates/res/templates.html delete mode 100644 example/templates/src/TemplateTestingController.js diff --git a/example/templates/bundle.json b/example/templates/bundle.json deleted file mode 100644 index 38989c408c..0000000000 --- a/example/templates/bundle.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "extensions": { - "routes": [ - { - "when": "/templates", - "templateUrl": "templates.html" - } - ], - "controllers": [ - { - "implementation": "TemplateTestingController.js", - "key": "TTC", - "depends": [ "$scope", "objectService" ] - } - ] - } -} diff --git a/example/templates/res/templates.html b/example/templates/res/templates.html deleted file mode 100644 index ac2747acaa..0000000000 --- a/example/templates/res/templates.html +++ /dev/null @@ -1,14 +0,0 @@ -
-
-
mct-include
- - -
- -
-
mct-representation
- - - -
-
diff --git a/example/templates/src/TemplateTestingController.js b/example/templates/src/TemplateTestingController.js deleted file mode 100644 index ccb4223603..0000000000 --- a/example/templates/src/TemplateTestingController.js +++ /dev/null @@ -1,18 +0,0 @@ -/*global define*/ -define([], function () { - 'use strict'; - - return function TemplateTestingController($scope, objectService) { - $scope.ikey = ''; - $scope.rkey = ''; - $scope.rid = ''; - $scope.obj = undefined; - - $scope.$watch('rid', function (id) { - $scope.obj = undefined; - objectService.getObjects([id]).then(function (objs) { - $scope.obj = objs[id]; - }); - }); - }; -}); From 01b6fda1f2f89e27d06186f3e6b91d30200105cc Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 16:11:00 -0700 Subject: [PATCH 13/26] [Representation] Update spec for mct-include ...to account for usage of templateLinker to add or remove the whole element from the DOM when there is or is not a template to show. --- .../representation/test/MCTIncludeSpec.js | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/platform/representation/test/MCTIncludeSpec.js b/platform/representation/test/MCTIncludeSpec.js index 135988b509..43c31a4443 100644 --- a/platform/representation/test/MCTIncludeSpec.js +++ b/platform/representation/test/MCTIncludeSpec.js @@ -31,9 +31,20 @@ define( describe("The mct-include directive", function () { var testTemplates, - mockSce, + mockLinker, + mockScope, + mockElement, + mockChangeTemplate, mctInclude; + function fireWatch(expr, value) { + mockScope.$watch.calls.forEach(function (call) { + if (call.args[0] === expr) { + call.args[1](value); + } + }); + } + beforeEach(function () { testTemplates = [ { @@ -47,42 +58,37 @@ define( templateUrl: "z/template.html" } ]; - mockSce = jasmine.createSpyObj( - '$sce', - ['trustAsResourceUrl'] - ); - mockSce.trustAsResourceUrl.andCallFake(function (url) { - return url; - }); - mctInclude = new MCTInclude(testTemplates, mockSce); - }); - - it("has a built-in template, with ng-include src=inclusion", function () { - // Not rigorous, but should detect many cases when template is broken. - expect(mctInclude.template.indexOf("ng-include")).not.toEqual(-1); - expect(mctInclude.template.indexOf("inclusion")).not.toEqual(-1); + mockLinker = jasmine.createSpyObj('templateLinker', ['link']); + mockScope = jasmine.createSpyObj('$scope', ['$watch', '$on']); + mockElement = jasmine.createSpyObj('element', ['empty']); + mockChangeTemplate = jasmine.createSpy('changeTemplate'); + mockLinker.link.andReturn(mockChangeTemplate); + mctInclude = new MCTInclude(testTemplates, mockLinker); + mctInclude.link(mockScope, mockElement, {}); }); it("is restricted to elements", function () { expect(mctInclude.restrict).toEqual("E"); }); - it("reads a template location from a scope's key variable", function () { - var scope = { key: "abc" }; - mctInclude.controller(scope); - expect(scope.inclusion).toEqual("a/b/c/template.html"); - - scope = { key: "xyz" }; - mctInclude.controller(scope); - expect(scope.inclusion).toEqual("x/y/z/template.html"); + it("exposes templates via the templateLinker", function () { + expect(mockLinker.link) + .toHaveBeenCalledWith(mockScope, mockElement); }); - it("trusts template URLs", function () { - mctInclude.controller({ key: "xyz" }); - expect(mockSce.trustAsResourceUrl) + it("reads a template location from a scope's key variable", function () { + mockScope.key = 'abc'; + fireWatch('key', mockScope.key); + expect(mockChangeTemplate) + .toHaveBeenCalledWith("a/b/c/template.html"); + + mockScope.key = 'xyz'; + fireWatch('key', mockScope.key); + expect(mockChangeTemplate) .toHaveBeenCalledWith("x/y/z/template.html"); }); + }); } ); From ca62cc9066d6e4e7c9e68e204ce2da575d0d72b1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 16:35:08 -0700 Subject: [PATCH 14/26] [Representation] Update spec for mct-representation ...to account for usage of templateLinker to add or remove the whole element from the DOM when there is or is not a template to show. --- .../test/MCTRepresentationSpec.js | 90 ++++++++++--------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/platform/representation/test/MCTRepresentationSpec.js b/platform/representation/test/MCTRepresentationSpec.js index a50347df70..1f2d55b4ab 100644 --- a/platform/representation/test/MCTRepresentationSpec.js +++ b/platform/representation/test/MCTRepresentationSpec.js @@ -38,8 +38,9 @@ define( testViews, mockRepresenters, mockQ, - mockSce, + mockLinker, mockLog, + mockChangeTemplate, mockScope, mockElement, mockDomainObject, @@ -54,6 +55,14 @@ define( }; } + function fireWatch(expr, value) { + mockScope.$watch.calls.forEach(function (call) { + if (call.args[0] === expr) { + call.args[1](value); + } + }); + } + beforeEach(function () { testRepresentations = [ { @@ -96,45 +105,38 @@ define( }); mockQ = { when: mockPromise }; - mockSce = jasmine.createSpyObj( - '$sce', - ['trustAsResourceUrl'] - ); + mockLinker = jasmine.createSpyObj('templateLinker', ['link']); + mockChangeTemplate = jasmine.createSpy('changeTemplate'); mockLog = jasmine.createSpyObj("$log", LOG_FUNCTIONS); - - mockSce.trustAsResourceUrl.andCallFake(function (url) { - return url; - }); mockScope = jasmine.createSpyObj("scope", [ "$watch", "$on" ]); mockElement = jasmine.createSpyObj("element", JQLITE_FUNCTIONS); mockDomainObject = jasmine.createSpyObj("domainObject", DOMAIN_OBJECT_METHODS); mockDomainObject.getModel.andReturn(testModel); + mockLinker.link.andReturn(mockChangeTemplate); mctRepresentation = new MCTRepresentation( testRepresentations, testViews, mockRepresenters, mockQ, - mockSce, + mockLinker, mockLog ); - }); - - - it("has a built-in template, with ng-include src=inclusion", function () { - // Not rigorous, but should detect many cases when template is broken. - expect(mctRepresentation.template.indexOf("ng-include")).not.toEqual(-1); - expect(mctRepresentation.template.indexOf("inclusion")).not.toEqual(-1); + mctRepresentation.link(mockScope, mockElement); }); it("is restricted to elements", function () { expect(mctRepresentation.restrict).toEqual("E"); }); + it("exposes templates via the templateLinker", function () { + expect(mockLinker.link) + .toHaveBeenCalledWith(mockScope, mockElement); + }); + it("watches scope when linked", function () { - mctRepresentation.link(mockScope, mockElement); expect(mockScope.$watch).toHaveBeenCalledWith( "key", jasmine.any(Function) @@ -150,42 +152,46 @@ define( }); it("recognizes keys for representations", function () { - mctRepresentation.link(mockScope, mockElement); - mockScope.key = "abc"; + mockScope.domainObject = mockDomainObject; // Trigger the watch - mockScope.$watch.calls[0].args[1](); + fireWatch('key', mockScope.key); + fireWatch('domainObject', mockDomainObject); - expect(mockScope.inclusion).toEqual("a/b/c/template.html"); + expect(mockChangeTemplate) + .toHaveBeenCalledWith("a/b/c/template.html"); }); it("recognizes keys for views", function () { - mctRepresentation.link(mockScope, mockElement); - mockScope.key = "xyz"; + mockScope.domainObject = mockDomainObject; - // Trigger the watch - mockScope.$watch.calls[0].args[1](); + // Trigger the watches + fireWatch('key', mockScope.key); + fireWatch('domainObject', mockDomainObject); - expect(mockScope.inclusion).toEqual("x/y/z/template.html"); - }); - - it("trusts template URLs", function () { - mctRepresentation.link(mockScope, mockElement); - - mockScope.key = "xyz"; - - // Trigger the watch - mockScope.$watch.calls[0].args[1](); - - expect(mockSce.trustAsResourceUrl) + expect(mockChangeTemplate) .toHaveBeenCalledWith("x/y/z/template.html"); }); - it("loads declared capabilities", function () { - mctRepresentation.link(mockScope, mockElement); + it("does not load templates until there is an object", function () { + mockScope.key = "xyz"; + // Trigger the watch + fireWatch('key', mockScope.key); + + expect(mockChangeTemplate) + .not.toHaveBeenCalledWith(jasmine.any(String)); + + mockScope.domainObject = mockDomainObject; + fireWatch('domainObject', mockDomainObject); + + expect(mockChangeTemplate) + .toHaveBeenCalledWith(jasmine.any(String)); + }); + + it("loads declared capabilities", function () { mockScope.key = "def"; mockScope.domainObject = mockDomainObject; @@ -199,8 +205,6 @@ define( }); it("logs when no representation is available for a key", function () { - mctRepresentation.link(mockScope, mockElement); - mockScope.key = "someUnknownThing"; // Verify precondition @@ -214,8 +218,6 @@ define( }); it("clears out obsolete peroperties from scope", function () { - mctRepresentation.link(mockScope, mockElement); - mockScope.key = "def"; mockScope.domainObject = mockDomainObject; mockDomainObject.useCapability.andReturn("some value"); From e7e66bff4b418a0cbc0b386ec0dfd06fdfb735fb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 28 Oct 2015 16:49:19 -0700 Subject: [PATCH 15/26] [Representation] Begin adding TemplateLinker spec --- .../representation/test/TemplateLinkerSpec.js | 95 +++++++++++++++++++ platform/representation/test/suite.json | 5 +- 2 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 platform/representation/test/TemplateLinkerSpec.js diff --git a/platform/representation/test/TemplateLinkerSpec.js b/platform/representation/test/TemplateLinkerSpec.js new file mode 100644 index 0000000000..6f525ce73f --- /dev/null +++ b/platform/representation/test/TemplateLinkerSpec.js @@ -0,0 +1,95 @@ +/***************************************************************************** + * Open MCT Web, Copyright (c) 2014-2015, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT Web 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 Web 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. + *****************************************************************************/ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/TemplateLinker"], + function (TemplateLinker) { + 'use strict'; + + var JQLITE_METHODS = [ 'replaceWith', 'empty' ]; + + describe("TemplateLinker", function () { + var mockHttp, + mockCompile, + mockLog, + mockScope, + mockElement, + mockTemplates, + mockElements, + linker; + + beforeEach(function () { + mockHttp = jasmine.createSpyObj('$http', ['get']); + mockCompile = jasmine.createSpy('$compile'); + mockLog = jasmine.createSpyObj('$log', ['error', 'warn']); + mockScope = jasmine.createSpyObj('$scope', ['$on']); + mockElement = jasmine.createSpyObj('element', JQLITE_METHODS); + mockTemplates = {}; + mockElements = {}; + + mockCompile.andCallFake(function (html) { + mockTemplates[html] = jasmine.createSpy('template'); + mockElements[html] = + jasmine.createSpyObj('templateEl', JQLITE_METHODS); + mockTemplates[html].andReturn(mockElements[html]); + return mockTemplates[html]; + }); + + linker = new TemplateLinker( + mockHttp, + mockCompile, + mockLog + ); + }); + + describe("when linking elements", function () { + var changeTemplate, + commentElement; + + function findCommentElement() { + mockCompile.calls.forEach(function (call) { + var html = call.args[0]; + if (html.indexOf("')(scope), @@ -125,7 +125,11 @@ define( } } - removeElement(); + if (templateUrl) { + changeTemplate(templateUrl); + } else { + removeElement(); + } return changeTemplate; }; From a48370abd3ae20878ec5068681292bc8ceabbd75 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 29 Oct 2015 08:18:13 -0700 Subject: [PATCH 20/26] [Representation] Update spec --- platform/representation/test/MCTIncludeSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/representation/test/MCTIncludeSpec.js b/platform/representation/test/MCTIncludeSpec.js index 43c31a4443..68fa7f5f77 100644 --- a/platform/representation/test/MCTIncludeSpec.js +++ b/platform/representation/test/MCTIncludeSpec.js @@ -73,7 +73,7 @@ define( it("exposes templates via the templateLinker", function () { expect(mockLinker.link) - .toHaveBeenCalledWith(mockScope, mockElement); + .toHaveBeenCalledWith(mockScope, mockElement, undefined); }); it("reads a template location from a scope's key variable", function () { From 5b475c9f641c692555548f71b997b1b46d95a155 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 29 Oct 2015 08:27:20 -0700 Subject: [PATCH 21/26] [Representation] Add more test cases ...for TemplateLinker. --- .../representation/test/TemplateLinkerSpec.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/platform/representation/test/TemplateLinkerSpec.js b/platform/representation/test/TemplateLinkerSpec.js index 57d1300e84..fd8de94931 100644 --- a/platform/representation/test/TemplateLinkerSpec.js +++ b/platform/representation/test/TemplateLinkerSpec.js @@ -202,6 +202,40 @@ define( }); }); + + describe("when an initial template URL is provided", function () { + var testUrl; + + beforeEach(function () { + testUrl = "some/test/url.html"; + linker.link(mockScope, mockElement, testUrl); + }); + + it("does not remove the element initially", function () { + expect(mockElement.replaceWith) + .not.toHaveBeenCalled(); + }); + + it("loads the specified template", function () { + expect(mockHttp.get).toHaveBeenCalledWith(testUrl); + }); + }); + + it("does not issue multiple requests for the same URL", function () { + var testUrls = ['a', 'b', 'c', 'd'].map(function (k) { + return k + "/some/template.html"; + }); + + testUrls.forEach(function (testUrl, i) { + expect(mockHttp.get.callCount).toEqual(i); + linker.link(mockScope, mockElement, testUrl); + expect(mockHttp.get.callCount).toEqual(i + 1); + linker.link(mockScope, mockElement, testUrl); + expect(mockHttp.get.callCount).toEqual(i + 1); + linker.link(mockScope, mockElement)(testUrl); + expect(mockHttp.get.callCount).toEqual(i + 1); + }); + }); }); } ); From 5af3d575a200170a351beb9f3e211d646f5fa888 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 30 Oct 2015 10:44:48 -0700 Subject: [PATCH 22/26] [Representation] Remove redundant listener The event is already listened-for at the representation level to trigger cleanup, including gestures, so the info gesture does not need to listen for it as well. Per code review feedback, nasa/openmctweb#222 --- platform/commonUI/inspect/src/gestures/InfoGesture.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/platform/commonUI/inspect/src/gestures/InfoGesture.js b/platform/commonUI/inspect/src/gestures/InfoGesture.js index 6be25e646d..688a27cb6c 100644 --- a/platform/commonUI/inspect/src/gestures/InfoGesture.js +++ b/platform/commonUI/inspect/src/gestures/InfoGesture.js @@ -126,13 +126,6 @@ define( this.pendingBubble = this.$timeout(displayBubble, this.delay); this.element.on('mouseleave', this.hideBubbleCallback); - - // Also make sure we dismiss bubble if representation is destroyed - // before the mouse actually leaves it - this.scopeOff = - this.element.scope() && - this.element.scope().$on('$destroy', this.hideBubbleCallback); - }; @@ -145,9 +138,6 @@ define( this.hideBubble(); // ...and detach listeners this.element.off('mouseenter', this.showBubbleCallback); - if (this.scopeOff) { - this.scopeOff(); - } }; return InfoGesture; From 5ed34c1c3042cd09faeecb5261599e7f997d143f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 30 Oct 2015 11:02:13 -0700 Subject: [PATCH 23/26] [Representation] Build URLs from templateLinker --- platform/representation/src/MCTInclude.js | 10 +++------- .../representation/src/MCTRepresentation.js | 6 +----- platform/representation/src/TemplateLinker.js | 14 +++++++++++++ .../representation/test/MCTIncludeSpec.js | 18 +++++++++++++---- .../test/MCTRepresentationSpec.js | 20 ++++++++++++++++--- .../representation/test/TemplateLinkerSpec.js | 10 ++++++++++ 6 files changed, 59 insertions(+), 19 deletions(-) diff --git a/platform/representation/src/MCTInclude.js b/platform/representation/src/MCTInclude.js index c7b3c9a3ed..761a798dfa 100644 --- a/platform/representation/src/MCTInclude.js +++ b/platform/representation/src/MCTInclude.js @@ -71,14 +71,10 @@ define( // Prepopulate templateMap for easy look up by key templates.forEach(function (template) { - var key = template.key, - path = [ - template.bundle.path, - template.bundle.resources, - template.templateUrl - ].join("/"); + var key = template.key; // First found should win (priority ordering) - templateMap[key] = templateMap[key] || path; + templateMap[key] = + templateMap[key] || templateLinker.getPath(template); }); return { diff --git a/platform/representation/src/MCTRepresentation.js b/platform/representation/src/MCTRepresentation.js index 4704c0265e..10b6d3ccec 100644 --- a/platform/representation/src/MCTRepresentation.js +++ b/platform/representation/src/MCTRepresentation.js @@ -72,11 +72,7 @@ define( // Get a path to a representation function getPath(representation) { - return [ - representation.bundle.path, - representation.bundle.resources, - representation.templateUrl - ].join("/"); + return templateLinker.getPath(representation); } // Look up a matching representation for this domain object diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index e2b9f42155..da357fc1e3 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -61,6 +61,20 @@ define( })); }; + /** + * Get a path to a template from an extension definition fo + * a template, representation, or view. + * @param {TemplateDefinition} extensionDefinition the definition + * of the template/representation/view to resolve + */ + TemplateLinker.prototype.getPath = function (extensionDefinition) { + return [ + extensionDefinition.bundle.path, + extensionDefinition.bundle.resources, + extensionDefinition.templateUrl + ].join('/'); + }; + /** * Populate the given element with templates, within the given scope; * intended to support the `link` function of the supported directives. diff --git a/platform/representation/test/MCTIncludeSpec.js b/platform/representation/test/MCTIncludeSpec.js index 68fa7f5f77..94dc62fd0a 100644 --- a/platform/representation/test/MCTIncludeSpec.js +++ b/platform/representation/test/MCTIncludeSpec.js @@ -31,6 +31,7 @@ define( describe("The mct-include directive", function () { var testTemplates, + testUrls, mockLinker, mockScope, mockElement, @@ -58,11 +59,21 @@ define( templateUrl: "z/template.html" } ]; - mockLinker = jasmine.createSpyObj('templateLinker', ['link']); + testUrls = {}; + testTemplates.forEach(function (t, i) { + testUrls[t.key] = "some URL " + String(i); + }); + mockLinker = jasmine.createSpyObj( + 'templateLinker', + ['link', 'getPath'] + ); mockScope = jasmine.createSpyObj('$scope', ['$watch', '$on']); mockElement = jasmine.createSpyObj('element', ['empty']); mockChangeTemplate = jasmine.createSpy('changeTemplate'); mockLinker.link.andReturn(mockChangeTemplate); + mockLinker.getPath.andCallFake(function (template) { + return testUrls[template.key]; + }); mctInclude = new MCTInclude(testTemplates, mockLinker); mctInclude.link(mockScope, mockElement, {}); }); @@ -80,15 +91,14 @@ define( mockScope.key = 'abc'; fireWatch('key', mockScope.key); expect(mockChangeTemplate) - .toHaveBeenCalledWith("a/b/c/template.html"); + .toHaveBeenCalledWith(testUrls.abc); mockScope.key = 'xyz'; fireWatch('key', mockScope.key); expect(mockChangeTemplate) - .toHaveBeenCalledWith("x/y/z/template.html"); + .toHaveBeenCalledWith(testUrls.xyz); }); - }); } ); diff --git a/platform/representation/test/MCTRepresentationSpec.js b/platform/representation/test/MCTRepresentationSpec.js index 1f2d55b4ab..30fed7c0ca 100644 --- a/platform/representation/test/MCTRepresentationSpec.js +++ b/platform/representation/test/MCTRepresentationSpec.js @@ -36,6 +36,7 @@ define( describe("The mct-representation directive", function () { var testRepresentations, testViews, + testUrls, mockRepresenters, mockQ, mockLinker, @@ -64,6 +65,8 @@ define( } beforeEach(function () { + testUrls = {}; + testRepresentations = [ { key: "abc", @@ -94,6 +97,11 @@ define( testModel = { someKey: "some value" }; + testUrls = {}; + testViews.concat(testRepresentations).forEach(function (t, i) { + testUrls[t.key] = "some URL " + String(i); + }); + mockRepresenters = ["A", "B"].map(function (name) { var constructor = jasmine.createSpy("Representer" + name), representer = jasmine.createSpyObj( @@ -105,7 +113,10 @@ define( }); mockQ = { when: mockPromise }; - mockLinker = jasmine.createSpyObj('templateLinker', ['link']); + mockLinker = jasmine.createSpyObj( + 'templateLinker', + ['link', 'getPath'] + ); mockChangeTemplate = jasmine.createSpy('changeTemplate'); mockLog = jasmine.createSpyObj("$log", LOG_FUNCTIONS); @@ -115,6 +126,9 @@ define( mockDomainObject.getModel.andReturn(testModel); mockLinker.link.andReturn(mockChangeTemplate); + mockLinker.getPath.andCallFake(function (ext) { + return testUrls[ext.key]; + }); mctRepresentation = new MCTRepresentation( testRepresentations, @@ -160,7 +174,7 @@ define( fireWatch('domainObject', mockDomainObject); expect(mockChangeTemplate) - .toHaveBeenCalledWith("a/b/c/template.html"); + .toHaveBeenCalledWith(testUrls.abc); }); it("recognizes keys for views", function () { @@ -172,7 +186,7 @@ define( fireWatch('domainObject', mockDomainObject); expect(mockChangeTemplate) - .toHaveBeenCalledWith("x/y/z/template.html"); + .toHaveBeenCalledWith(testUrls.xyz); }); it("does not load templates until there is an object", function () { diff --git a/platform/representation/test/TemplateLinkerSpec.js b/platform/representation/test/TemplateLinkerSpec.js index fd8de94931..38a3f867e2 100644 --- a/platform/representation/test/TemplateLinkerSpec.js +++ b/platform/representation/test/TemplateLinkerSpec.js @@ -69,6 +69,16 @@ define( ); }); + it("resolves extension paths", function () { + expect(linker.getPath({ + bundle: { + path: 'a', + resources: 'b' + }, + templateUrl: 'c/d.html' + })).toEqual('a/b/c/d.html'); + }); + describe("when linking elements", function () { var changeTemplate, commentElement; From d5f1d45759b24604019220d3c139d15722fca5d4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 30 Oct 2015 12:22:43 -0700 Subject: [PATCH 24/26] [Representation] Use $templateRequest ...from templateLinker, to remove the need to use $http and to explicitly cache templates. --- platform/representation/bundle.json | 2 +- platform/representation/src/TemplateLinker.js | 41 +++----- .../representation/test/TemplateLinkerSpec.js | 94 ++++++------------- 3 files changed, 43 insertions(+), 94 deletions(-) diff --git a/platform/representation/bundle.json b/platform/representation/bundle.json index 7d6ddd3113..8b185422b9 100644 --- a/platform/representation/bundle.json +++ b/platform/representation/bundle.json @@ -52,7 +52,7 @@ { "key": "templateLinker", "implementation": "TemplateLinker.js", - "depends": [ "$http", "$compile", "$log" ], + "depends": [ "$templateRequest", "$sce", "$compile", "$log" ], "comment": "For internal use by mct-include and mct-representation." } ], diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index da357fc1e3..b462afa626 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -39,26 +39,18 @@ define( * @param $log Angular's `$log` service * @private */ - function TemplateLinker($http, $compile, $log) { - this.templateMap = {}; - this.$http = $http; + function TemplateLinker($templateRequest, $sce, $compile, $log) { + this.$templateRequest = $templateRequest; + this.$sce = $sce; this.$compile = $compile; this.$log = $log; } TemplateLinker.prototype.load = function (templateUrl) { - var $http = this.$http, - $compile = this.$compile, - $log = this.$log, - templateMap = this.templateMap; - - return (templateMap[templateUrl] = templateMap[templateUrl] || - $http.get(templateUrl).then(function (response) { - return $compile(response.data); - }, function () { - $log.warn("Couldn't load template at " + templateUrl); - templateMap[templateUrl] = undefined; - })); + return this.$templateRequest( + this.$sce.trustAsResourceUrl(templateUrl), + false + ); }; /** @@ -108,18 +100,13 @@ define( } function populateElement(template) { - template(scope, function (innerClone) { - element.empty(); - element.append(innerClone); - }); + element.empty(); + element.append(self.$compile(template)(scope)); } - function applyTemplate(template, templateUrl) { - if (template) { - populateElement(template); - } else { - removeElement(); - } + function badTemplate(templateUrl) { + self.$log.warn("Couldn't load template at " + templateUrl); + removeElement(); } function changeTemplate(templateUrl) { @@ -129,8 +116,10 @@ define( self.load(templateUrl).then(function (template) { // Avoid race conditions if (templateUrl === activeTemplateUrl) { - applyTemplate(template); + populateElement(template); } + }, function () { + badTemplate(templateUrl); }); } else { removeElement(); diff --git a/platform/representation/test/TemplateLinkerSpec.js b/platform/representation/test/TemplateLinkerSpec.js index 38a3f867e2..bf762fba53 100644 --- a/platform/representation/test/TemplateLinkerSpec.js +++ b/platform/representation/test/TemplateLinkerSpec.js @@ -30,30 +30,29 @@ define( var JQLITE_METHODS = [ 'replaceWith', 'empty', 'append' ]; describe("TemplateLinker", function () { - var mockHttp, + var mockTemplateRequest, + mockSce, mockCompile, mockLog, mockScope, mockElement, mockTemplates, mockElements, - mockHttpPromise, - mockChainPromise, + mockPromise, linker; beforeEach(function () { - mockHttp = jasmine.createSpyObj('$http', ['get']); + mockTemplateRequest = jasmine.createSpy('$templateRequest'); + mockSce = jasmine.createSpyObj('$sce', ['trustAsResourceUrl']); mockCompile = jasmine.createSpy('$compile'); mockLog = jasmine.createSpyObj('$log', ['error', 'warn']); mockScope = jasmine.createSpyObj('$scope', ['$on']); mockElement = jasmine.createSpyObj('element', JQLITE_METHODS); - mockHttpPromise = jasmine.createSpyObj('promise1', ['then']); - mockChainPromise = jasmine.createSpyObj('promise2', ['then']); + mockPromise = jasmine.createSpyObj('promise', ['then']); mockTemplates = {}; mockElements = {}; - mockHttp.get.andReturn(mockHttpPromise); - mockHttpPromise.then.andReturn(mockChainPromise); + mockTemplateRequest.andReturn(mockPromise); mockCompile.andCallFake(function (html) { mockTemplates[html] = jasmine.createSpy('template'); mockElements[html] = @@ -61,9 +60,13 @@ define( mockTemplates[html].andReturn(mockElements[html]); return mockTemplates[html]; }); + mockSce.trustAsResourceUrl.andCallFake(function (url) { + return { trusted: url }; + }); linker = new TemplateLinker( - mockHttp, + mockTemplateRequest, + mockSce, mockCompile, mockLog ); @@ -118,44 +121,28 @@ define( testUrl = "some/url/template.html"; testTemplate = "
Some template!
"; changeTemplate(testUrl); + mockPromise.then.mostRecentCall + .args[0](testTemplate); }); - it("loads templates using $http", function () { - expect(mockHttp.get).toHaveBeenCalledWith(testUrl); + it("loads templates using $templateRequest", function () { + expect(mockTemplateRequest).toHaveBeenCalledWith({ + trusted: testUrl + }, false); }); it("compiles loaded templates with linked scope", function () { - var chainValue; - chainValue = mockHttpPromise.then.mostRecentCall.args[0]({ - data: testTemplate - }); expect(mockCompile).toHaveBeenCalledWith(testTemplate); - mockChainPromise.then.mostRecentCall.args[0](chainValue); - expect(mockTemplates[testTemplate]).toHaveBeenCalledWith( - mockScope, - jasmine.any(Function) - ); + expect(mockTemplates[testTemplate]) + .toHaveBeenCalledWith(mockScope); }); it("replaces comments with specified element", function () { - mockChainPromise.then.mostRecentCall.args[0]( - mockHttpPromise.then.mostRecentCall.args[0]({ - data: testTemplate - }) - ); expect(commentElement.replaceWith) .toHaveBeenCalledWith(mockElement); }); it("appends rendered content to the specified element", function () { - mockChainPromise.then.mostRecentCall.args[0]( - mockHttpPromise.then.mostRecentCall.args[0]({ - data: testTemplate - }) - ); - mockTemplates[testTemplate].mostRecentCall.args[1]( - mockElements[testTemplate] - ); expect(mockElement.append) .toHaveBeenCalledWith(mockElements[testTemplate]); }); @@ -163,11 +150,6 @@ define( it("clears templates when called with undefined", function () { expect(mockElement.replaceWith.callCount) .toEqual(1); - mockChainPromise.then.mostRecentCall.args[0]( - mockHttpPromise.then.mostRecentCall.args[0]({ - data: testTemplate - }) - ); changeTemplate(undefined); expect(mockElement.replaceWith.callCount) .toEqual(2); @@ -175,24 +157,15 @@ define( .toEqual(commentElement); }); - it("logs no warnings", function () { - mockChainPromise.then.mostRecentCall.args[0]( - mockHttpPromise.then.mostRecentCall.args[0]({ - data: testTemplate - }) - ); - mockTemplates[testTemplate].mostRecentCall.args[1]( - mockElements[testTemplate] - ); + it("logs no warnings for nominal changes", function () { expect(mockLog.warn).not.toHaveBeenCalled(); }); describe("which cannot be found", function () { beforeEach(function () { - mockChainPromise.then.mostRecentCall.args[0]( - // Reject the http promise - mockHttpPromise.then.mostRecentCall.args[1]() - ); + changeTemplate("some/bad/url"); + // Reject the template promise + mockPromise.then.mostRecentCall.args[1](); }); it("removes the element from the DOM", function () { @@ -227,25 +200,12 @@ define( }); it("loads the specified template", function () { - expect(mockHttp.get).toHaveBeenCalledWith(testUrl); + expect(mockTemplateRequest).toHaveBeenCalledWith({ + trusted: testUrl + }, false); }); }); - it("does not issue multiple requests for the same URL", function () { - var testUrls = ['a', 'b', 'c', 'd'].map(function (k) { - return k + "/some/template.html"; - }); - - testUrls.forEach(function (testUrl, i) { - expect(mockHttp.get.callCount).toEqual(i); - linker.link(mockScope, mockElement, testUrl); - expect(mockHttp.get.callCount).toEqual(i + 1); - linker.link(mockScope, mockElement, testUrl); - expect(mockHttp.get.callCount).toEqual(i + 1); - linker.link(mockScope, mockElement)(testUrl); - expect(mockHttp.get.callCount).toEqual(i + 1); - }); - }); }); } ); From a38d4829ebaa6f048d9077b2bef0410e6f3d814c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 30 Oct 2015 12:38:32 -0700 Subject: [PATCH 25/26] [Representation] Add JSDoc --- platform/representation/src/TemplateLinker.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index b462afa626..f3824c797f 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -46,6 +46,14 @@ define( this.$log = $log; } + /** + * Load a template from the given URL. This request will be handled + * via `$templateRequest` to ensure caching et cetera. + * @param {string} the URL for the template + * @returns {Promise.} a promise for the HTML content of + * the template + * @private + */ TemplateLinker.prototype.load = function (templateUrl) { return this.$templateRequest( this.$sce.trustAsResourceUrl(templateUrl), From 4abb48abd8b01856328f6fa770e1abf1ba202bf5 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 30 Oct 2015 13:34:13 -0700 Subject: [PATCH 26/26] [Representation] Update parameters in JSDoc --- platform/representation/src/TemplateLinker.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/platform/representation/src/TemplateLinker.js b/platform/representation/src/TemplateLinker.js index f3824c797f..426bbbe318 100644 --- a/platform/representation/src/TemplateLinker.js +++ b/platform/representation/src/TemplateLinker.js @@ -34,7 +34,9 @@ define( * and/or removing that element from the DOM when there is no * template to populate it with. * - * @param $http Angular's `$http` service + * @param {Function} $templateRequest Angular's `$templateRequest` + * service + * @param $sce Angular's `$sce` service * @param {Function} $compile Angular's `$compile` service * @param $log Angular's `$log` service * @private