From e67a2e63cf73c709ef6dbbac05e35df0a40341b2 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 23 Sep 2015 13:59:05 -0700 Subject: [PATCH 01/28] [Readme] Add notes on building documentation Add notes to README about building documentation; in particular, document the need to install libcairo. --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index 42cd060282..8b982a8bb0 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,18 @@ This build will: Run as `mvn clean install`. +### Building Documentation + +Open MCT Web's documentation is generated by an +[npm](https://www.npmjs.com/)-based build: + +* `npm install` _(only needs to run once)_ +* `npm run docs` + +Documentation will be generated in `target/docs`. Note that diagram +generation is dependent on having [Cairo](http://cairographics.org/download/) +installed. + # Glossary Certain terms are used throughout Open MCT Web with consistent meanings From 7dc13dab66c8bafe8ba23079b7d8ee3649894de0 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 23 Sep 2015 14:37:49 -0700 Subject: [PATCH 02/28] [Readme] Add link to node-canvas --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8b982a8bb0..6a412412ef 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,10 @@ Open MCT Web's documentation is generated by an Documentation will be generated in `target/docs`. Note that diagram generation is dependent on having [Cairo](http://cairographics.org/download/) -installed. +installed; see +[node-canvas](https://github.com/Automattic/node-canvas#installation)'s +documentation for help with installation. + # Glossary From 4f0e8ada2f393a6759347af4f376b8d91f9b738b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 25 Sep 2015 10:47:41 -0700 Subject: [PATCH 03/28] [Process] Loosen commit message standards Loosen commit message standards, to minimize clutter in GitHub issues. nasa/openmctweb#145. --- CONTRIBUTING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5f2fb18d84..fcd1504ef0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -238,6 +238,9 @@ Commit messages should: * Contain a reference to a relevant issue number in the body of the commit. * This is important for traceability; while branch names also provide this, you cannot tell from looking at a commit what branch it was authored on. + * This may be omitted if the relevant issue is otherwise obvious from the + commit history (that is, if using `git log` from the relevant commit + directly leads to a similar issue reference) to minimize clutter. * Describe the change that was made, and any useful rationale therefore. * Comments in code should explain what things do, commit messages describe how they came to be done that way. From f5a4a370f9c5a3123552b620ae88586c359b9d7c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 25 Sep 2015 11:36:48 -0700 Subject: [PATCH 04/28] [Persistence] Add persisted timestamp ...to any domain object models loaded from persistence which do not have one. The presence of this timestamp is necessary for the persistence capability to determine whether an object should be created or updated when a request to persist is made. nasa/openmctweb#139. --- platform/core/bundle.json | 1 + .../core/src/models/PersistedModelProvider.js | 23 ++++++- .../test/models/PersistedModelProviderSpec.js | 65 +++++++++++++++---- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/platform/core/bundle.json b/platform/core/bundle.json index 952daf5570..330ac1c2b9 100644 --- a/platform/core/bundle.json +++ b/platform/core/bundle.json @@ -66,6 +66,7 @@ "depends": [ "persistenceService", "$q", + "now", "PERSISTENCE_SPACE", "ADDITIONAL_PERSISTENCE_SPACES" ] diff --git a/platform/core/src/models/PersistedModelProvider.js b/platform/core/src/models/PersistedModelProvider.js index a10f818179..c5e2927a96 100644 --- a/platform/core/src/models/PersistedModelProvider.js +++ b/platform/core/src/models/PersistedModelProvider.js @@ -39,14 +39,16 @@ define( * @param {PersistenceService} persistenceService the service in which * domain object models are persisted. * @param $q Angular's $q service, for working with promises + * @param {function} now a function which provides the current time * @param {string} space the name of the persistence space(s) * from which models should be retrieved. * @param {string} spaces additional persistence spaces to use */ - function PersistedModelProvider(persistenceService, $q, space, spaces) { + function PersistedModelProvider(persistenceService, $q, now, space, spaces) { this.persistenceService = persistenceService; this.$q = $q; this.spaces = [space].concat(spaces || []); + this.now = now; } // Take the most recently modified model, for cases where @@ -61,7 +63,9 @@ define( PersistedModelProvider.prototype.getModels = function (ids) { var persistenceService = this.persistenceService, $q = this.$q, - spaces = this.spaces; + spaces = this.spaces, + space = this.space, + now = this.now; // Load a single object model from any persistence spaces function loadModel(id) { @@ -72,11 +76,24 @@ define( }); } + // Ensure that models read from persistence have some + // sensible timestamp indicating they've been persisted. + function addPersistedTimestamp(model) { + if (model && (model.persisted === undefined)) { + model.persisted = model.modified !== undefined ? + model.modified : now(); + } + + return model; + } + // Package the result as id->model function packageResult(models) { var result = {}; ids.forEach(function (id, index) { - result[id] = models[index]; + if (models[index]) { + result[id] = addPersistedTimestamp(models[index]); + } }); return result; } diff --git a/platform/core/test/models/PersistedModelProviderSpec.js b/platform/core/test/models/PersistedModelProviderSpec.js index 8dcb58a400..81769834bf 100644 --- a/platform/core/test/models/PersistedModelProviderSpec.js +++ b/platform/core/test/models/PersistedModelProviderSpec.js @@ -35,6 +35,7 @@ define( SPACE = "space0", spaces = [ "space1" ], modTimes, + mockNow, provider; function mockPromise(value) { @@ -55,19 +56,33 @@ define( beforeEach(function () { modTimes = {}; mockQ = { when: mockPromise, all: mockAll }; - mockPersistenceService = { - readObject: function (space, id) { + mockPersistenceService = jasmine.createSpyObj( + 'persistenceService', + [ + 'createObject', + 'readObject', + 'updateObject', + 'deleteObject', + 'listSpaces', + 'listObjects' + ] + ); + mockNow = jasmine.createSpy("now"); + + mockPersistenceService.readObject + .andCallFake(function (space, id) { return mockPromise({ space: space, id: id, - modified: (modTimes[space] || {})[id] + modified: (modTimes[space] || {})[id], + persisted: 0 }); - } - }; + }); provider = new PersistedModelProvider( mockPersistenceService, mockQ, + mockNow, SPACE, spaces ); @@ -81,12 +96,13 @@ define( }); expect(models).toEqual({ - a: { space: SPACE, id: "a" }, - x: { space: SPACE, id: "x" }, - zz: { space: SPACE, id: "zz" } + a: { space: SPACE, id: "a", persisted: 0 }, + x: { space: SPACE, id: "x", persisted: 0 }, + zz: { space: SPACE, id: "zz", persisted: 0 } }); }); + it("reads object models from multiple spaces", function () { var models; @@ -99,9 +115,36 @@ define( }); expect(models).toEqual({ - a: { space: SPACE, id: "a" }, - x: { space: 'space1', id: "x", modified: 12321 }, - zz: { space: SPACE, id: "zz" } + a: { space: SPACE, id: "a", persisted: 0 }, + x: { space: 'space1', id: "x", modified: 12321, persisted: 0 }, + zz: { space: SPACE, id: "zz", persisted: 0 } + }); + }); + + + it("ensures that persisted timestamps are present", function () { + var mockCallback = jasmine.createSpy("callback"), + testModels = { + a: { modified: 123, persisted: 1984, name: "A" }, + b: { persisted: 1977, name: "B" }, + c: { modified: 42, name: "C" }, + d: { name: "D" } + }; + + mockPersistenceService.readObject.andCallFake( + function (space, id) { + return mockPromise(testModels[id]); + } + ); + mockNow.andReturn(12321); + + provider.getModels(Object.keys(testModels)).then(mockCallback); + + expect(mockCallback).toHaveBeenCalledWith({ + a: { modified: 123, persisted: 1984, name: "A" }, + b: { persisted: 1977, name: "B" }, + c: { modified: 42, persisted: 42, name: "C" }, + d: { persisted: 12321, name: "D" } }); }); From 581dbacc9f6705f6418dcfb69e0c42df01c6e5ab Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 28 Sep 2015 15:20:18 -0700 Subject: [PATCH 05/28] [Build] Remove SNAPSHOT status Remove SNAPSHOT status to tag end of sprint, WTD-827. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a4d8e1cf35..27aa95e3b7 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ gov.nasa.arc.wtd open-mct-web Open MCT Web - 0.8.1-SNAPSHOT + 0.8.1 war From d6fe543c16ba32c637b2106aaf3da07e0e49929b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 28 Sep 2015 15:25:37 -0700 Subject: [PATCH 06/28] [Build] Bump version number Bump version number to start sprint Asimov, WTD-827. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 27aa95e3b7..3c1ece26b4 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ gov.nasa.arc.wtd open-mct-web Open MCT Web - 0.8.1 + 0.8.2-SNAPSHOT war From f198c281bc3141057aee349531e3ed0c3b6b919f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 28 Sep 2015 15:57:55 -0700 Subject: [PATCH 07/28] [Mobile] Suppress Create button on mobile nasa/openmctweb#157 --- platform/commonUI/browse/res/templates/browse.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/platform/commonUI/browse/res/templates/browse.html b/platform/commonUI/browse/res/templates/browse.html index e6255bcb55..9a1be7e771 100644 --- a/platform/commonUI/browse/res/templates/browse.html +++ b/platform/commonUI/browse/res/templates/browse.html @@ -28,7 +28,9 @@
- +
From 571beb8df254d3e1a38d9515d81cd01e009a25cc Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 1 Oct 2015 14:34:52 -0700 Subject: [PATCH 08/28] [Time Conductor] Add JSDoc to mct-popup Based on feedback from code review; WTD-1515 --- .../general/src/directives/MCTPopup.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/platform/commonUI/general/src/directives/MCTPopup.js b/platform/commonUI/general/src/directives/MCTPopup.js index 33860204c6..aa411684ae 100644 --- a/platform/commonUI/general/src/directives/MCTPopup.js +++ b/platform/commonUI/general/src/directives/MCTPopup.js @@ -27,6 +27,24 @@ define( var TEMPLATE = "
"; + /** + * The `mct-popup` directive may be used to display elements + * which "pop up" over other parts of the page. Typically, this is + * done in conjunction with an `ng-if` to control the visibility + * of the popup. + * + * Example of usage: + * + * + * These are the contents of the popup! + * + * + * @constructor + * @memberof platform/commonUI/general + * @param $window the window object (as injected by Angular) + * @param $document the jqLite-wrapped document + * @param $compile Angular's $compile service + */ function MCTPopup($window, $document, $compile) { function link(scope, element, attrs, ctrl, transclude) { var body = $document.find('body'), From 5104a7990a09c72024a317a612c80c2de6b82ebb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 1 Oct 2015 15:26:00 -0700 Subject: [PATCH 09/28] [Time Conductor] Add popupService ...to consolidate positioning of popups, based on commonality between InfoService and MCTPopup. nasa/openmctweb#104. --- .../general/src/services/PopupService.js | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 platform/commonUI/general/src/services/PopupService.js diff --git a/platform/commonUI/general/src/services/PopupService.js b/platform/commonUI/general/src/services/PopupService.js new file mode 100644 index 0000000000..61ec1d11d4 --- /dev/null +++ b/platform/commonUI/general/src/services/PopupService.js @@ -0,0 +1,135 @@ +/***************************************************************************** + * 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*/ + +define( + function () { + "use strict"; + + /** + * Displays popup elements at specific positions within the document. + * @memberof platform/commonUI/general + * @constructor + */ + function PopupService($document, $window) { + this.$document = $document; + this.$window = $window; + } + + /** + * Options controlling how the popup is displaed. + * + * @typedef PopupOptions + * @memberof platform/commonUI/general + * @property {number} [offsetX] the horizontal distance, in pixels, + * to offset the element in whichever direction it is + * displayed. Defaults to 0. + * @property {number} [offsetY] the vertical distance, in pixels, + * to offset the element in whichever direction it is + * displayed. Defaults to 0. + * @property {number} [marginX] the horizontal position, in pixels, + * after which to prefer to display the element to the left. + * If negative, this is relative to the right edge of the + * page. Defaults to half the window's width. + * @property {number} [marginY] the vertical position, in pixels, + * after which to prefer to display the element upward. + * If negative, this is relative to the right edge of the + * page. Defaults to half the window's height. + * @property {string} [leftClass] class to apply when shifting to the left + * @property {string} [rightClass] class to apply when shifting to the right + * @property {string} [upClass] class to apply when shifting upward + * @property {string} [downClass] class to apply when shifting downward + */ + + /** + * Display a popup at a particular location. The location chosen will + * be the corner of the element; the element will be positioned either + * to the left or the right of this point depending on available + * horizontal space, and will similarly be shifted upward or downward + * depending on available vertical space. + * + * @param element the jqLite-wrapped DOM element to pop up + * @param {number[]} position x,y position of the element, in + * pixel coordinates. + * @param {PopupOptions} [options] additional options to control + * positioning of the popup + * @returns {Function} a function that may be invoked to + * dismiss the info bubble + */ + PopupService.prototype.display = function (element, position, options) { + var $document = this.$document, + $window = this.$window, + body = $document.find('body'), + winDim = [ $window.innerWidth, $window.innerHeight ], + margin, + offset, + bubble; + + function applyClassOption(direction) { + if (options[direction + 'Class']) { + element.addClass(options[direction + 'Class']); + } + } + + // Defaults + options = options || {}; + offset = [ + options.offsetX !== undefined ? options.offsetX : 0, + options.offsetY !== undefined ? options.offsetY : 0 + ]; + margin = [ options.marginX, options.marginY ].map(function (m, i) { + return m === undefined ? (winDim[i] / 2) : + m < 0 ? (m + winDim[i]) : m; + }); + + // Position the element + element.css('position', 'absolute'); + + if (position[0] > margin[0]) { + element.css('right', (winDim[0] - position[0] + offset[0]) + 'px'); + applyClassOption('left'); + } else { + element.css('left', position[0] + offset[0] + 'px'); + applyClassOption('right'); + } + + if (position[1] > margin[1]) { + element.css('bottom', (winDim[1] - position[1] + offset[1]) + 'px'); + applyClassOption('up'); + } else { + element.css('top', position[1] + offset[1] + 'px'); + applyClassOption('down'); + } + + // Add the menu to the body + body.append(element); + + // Return a function to dismiss the bubble + return function () { + element.remove(); + }; + }; + + return PopupService; + } +); + From 3050b265fb5c4c91cf1ee69e7505f670b2706cb4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 1 Oct 2015 15:27:36 -0700 Subject: [PATCH 10/28] [Time Conductor] Expose popupService --- platform/commonUI/general/bundle.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/platform/commonUI/general/bundle.json b/platform/commonUI/general/bundle.json index d6614697a0..82a1ccfbe8 100644 --- a/platform/commonUI/general/bundle.json +++ b/platform/commonUI/general/bundle.json @@ -8,6 +8,11 @@ "key": "urlService", "implementation": "services/UrlService.js", "depends": [ "$location" ] + }, + { + "key": "popupService", + "implementation": "services/PopupService.js", + "depends": [ "$window", "$document" ] } ], "runs": [ From 6cbd3e5faeab6f04f17209354b37b9b9c66dff99 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 1 Oct 2015 16:25:33 -0700 Subject: [PATCH 11/28] [Time Conductor] Use popupService from infoService --- platform/commonUI/general/bundle.json | 2 +- .../general/src/services/PopupService.js | 14 +++-- platform/commonUI/inspect/bundle.json | 7 +-- .../commonUI/inspect/src/InfoConstants.js | 20 +++++-- .../inspect/src/services/InfoService.js | 58 +++++++------------ 5 files changed, 50 insertions(+), 51 deletions(-) diff --git a/platform/commonUI/general/bundle.json b/platform/commonUI/general/bundle.json index 82a1ccfbe8..8e367cc060 100644 --- a/platform/commonUI/general/bundle.json +++ b/platform/commonUI/general/bundle.json @@ -12,7 +12,7 @@ { "key": "popupService", "implementation": "services/PopupService.js", - "depends": [ "$window", "$document" ] + "depends": [ "$document", "$window" ] } ], "runs": [ diff --git a/platform/commonUI/general/src/services/PopupService.js b/platform/commonUI/general/src/services/PopupService.js index 61ec1d11d4..3c655c035b 100644 --- a/platform/commonUI/general/src/services/PopupService.js +++ b/platform/commonUI/general/src/services/PopupService.js @@ -69,7 +69,8 @@ define( * * @param element the jqLite-wrapped DOM element to pop up * @param {number[]} position x,y position of the element, in - * pixel coordinates. + * pixel coordinates. Negative values are interpreted as + * relative to the right or bottom of the window. * @param {PopupOptions} [options] additional options to control * positioning of the popup * @returns {Function} a function that may be invoked to @@ -90,6 +91,10 @@ define( } } + function adjustNegatives(value, index) { + return value < 0 ? (value + winDim[index]) : value; + } + // Defaults options = options || {}; offset = [ @@ -97,9 +102,10 @@ define( options.offsetY !== undefined ? options.offsetY : 0 ]; margin = [ options.marginX, options.marginY ].map(function (m, i) { - return m === undefined ? (winDim[i] / 2) : - m < 0 ? (m + winDim[i]) : m; - }); + return m === undefined ? (winDim[i] / 2) : m; + }).map(adjustNegatives); + + position = position.map(adjustNegatives); // Position the element element.css('position', 'absolute'); diff --git a/platform/commonUI/inspect/bundle.json b/platform/commonUI/inspect/bundle.json index bafeb851ef..ed6858f13e 100644 --- a/platform/commonUI/inspect/bundle.json +++ b/platform/commonUI/inspect/bundle.json @@ -45,13 +45,12 @@ "implementation": "services/InfoService.js", "depends": [ "$compile", - "$document", - "$window", "$rootScope", + "popupService", "agentService" ] } - ], + ], "constants": [ { "key": "INFO_HOVER_DELAY", @@ -66,4 +65,4 @@ } ] } -} \ No newline at end of file +} diff --git a/platform/commonUI/inspect/src/InfoConstants.js b/platform/commonUI/inspect/src/InfoConstants.js index 4927de870f..4475f58439 100644 --- a/platform/commonUI/inspect/src/InfoConstants.js +++ b/platform/commonUI/inspect/src/InfoConstants.js @@ -31,13 +31,21 @@ define({ BUBBLE_TEMPLATE: "" + + "class=\"bubble-container\">" + "" + "" + "", - // Pixel offset for bubble, to align arrow position - BUBBLE_OFFSET: [ 0, -26 ], - // Max width and margins allowed for bubbles; defined in /platform/commonUI/general/res/sass/_constants.scss - BUBBLE_MARGIN_LR: 10, - BUBBLE_MAX_WIDTH: 300 + // Options and classes for bubble + BUBBLE_OPTIONS: { + offsetX: 0, + offsetY: -26, + leftClass: 'arw-left', + rightClass: 'arw-right', + topClass: 'arw-top', + bottomClss: 'arw-btm' + }, + BUBBLE_MOBILE_POSITION: [ 0, -25 ], + // Max width and margins allowed for bubbles; defined in /platform/commonUI/general/res/sass/_constants.scss + BUBBLE_MARGIN_LR: 10, + BUBBLE_MAX_WIDTH: 300 }); diff --git a/platform/commonUI/inspect/src/services/InfoService.js b/platform/commonUI/inspect/src/services/InfoService.js index ff6d23cb56..cd3fa4087b 100644 --- a/platform/commonUI/inspect/src/services/InfoService.js +++ b/platform/commonUI/inspect/src/services/InfoService.js @@ -27,18 +27,18 @@ define( "use strict"; var BUBBLE_TEMPLATE = InfoConstants.BUBBLE_TEMPLATE, - OFFSET = InfoConstants.BUBBLE_OFFSET; + MOBILE_POSITION = InfoConstants.BUBBLE_MOBILE_POSITION, + OPTIONS = InfoConstants.BUBBLE_OPTIONS; /** * Displays informative content ("info bubbles") for the user. * @memberof platform/commonUI/inspect * @constructor */ - function InfoService($compile, $document, $window, $rootScope, agentService) { + function InfoService($compile, $rootScope, popupService, agentService) { this.$compile = $compile; - this.$document = $document; - this.$window = $window; this.$rootScope = $rootScope; + this.popupService = popupService; this.agentService = agentService; } @@ -55,53 +55,39 @@ define( */ InfoService.prototype.display = function (templateKey, title, content, position) { var $compile = this.$compile, - $document = this.$document, - $window = this.$window, $rootScope = this.$rootScope, - body = $document.find('body'), scope = $rootScope.$new(), - winDim = [$window.innerWidth, $window.innerHeight], - bubbleSpaceLR = InfoConstants.BUBBLE_MARGIN_LR + InfoConstants.BUBBLE_MAX_WIDTH, - goLeft = position[0] > (winDim[0] - bubbleSpaceLR), - goUp = position[1] > (winDim[1] / 2), + bubbleSpaceLR = InfoConstants.BUBBLE_MARGIN_LR + + InfoConstants.BUBBLE_MAX_WIDTH, + options, + dismissPopup, bubble; - + // Pass model & container parameters into the scope scope.bubbleModel = content; scope.bubbleTemplate = templateKey; - scope.bubbleLayout = (goUp ? 'arw-btm' : 'arw-top') + ' ' + - (goLeft ? 'arw-right' : 'arw-left'); scope.bubbleTitle = title; // Create the context menu bubble = $compile(BUBBLE_TEMPLATE)(scope); - // Position the bubble - bubble.css('position', 'absolute'); + options = Object.create(OPTIONS); + options.marginX = -bubbleSpaceLR; + + // On a phone, bubble takes up more screen real estate, + // so position it differently (toward the bottom) if (this.agentService.isPhone(navigator.userAgent)) { - bubble.css('right', '0px'); - bubble.css('left', '0px'); - bubble.css('top', 'auto'); - bubble.css('bottom', '25px'); - } else { - if (goLeft) { - bubble.css('right', (winDim[0] - position[0] + OFFSET[0]) + 'px'); - } else { - bubble.css('left', position[0] + OFFSET[0] + 'px'); - } - if (goUp) { - bubble.css('bottom', (winDim[1] - position[1] + OFFSET[1]) + 'px'); - } else { - bubble.css('top', position[1] + OFFSET[1] + 'px'); - } + position = MOBILE_POSITION; + options = {}; } - // Add the menu to the body - body.append(bubble); + dismissPopup = + this.popupService.display(bubble, position, options); - // Return a function to dismiss the bubble - return function () { - bubble.remove(); + // Return a function to dismiss the info bubble + return function dismiss() { + dismissPopup(); + scope.$destroy(); }; }; From dfe909d6b56e9cd57fe791db46e5f9a52bcf966c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 1 Oct 2015 16:59:12 -0700 Subject: [PATCH 12/28] [Time Conductor] Show appropriate arrow ...on info bubbles, when using bubbles shown via the popupService. --- .../commonUI/general/src/services/Popup.js | 89 +++++++++++++++++++ .../general/src/services/PopupService.js | 26 ++---- .../commonUI/inspect/src/InfoConstants.js | 9 +- .../inspect/src/services/InfoService.js | 14 ++- 4 files changed, 111 insertions(+), 27 deletions(-) create mode 100644 platform/commonUI/general/src/services/Popup.js diff --git a/platform/commonUI/general/src/services/Popup.js b/platform/commonUI/general/src/services/Popup.js new file mode 100644 index 0000000000..6029ca29cb --- /dev/null +++ b/platform/commonUI/general/src/services/Popup.js @@ -0,0 +1,89 @@ +/***************************************************************************** + * 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*/ + +define( + function () { + "use strict"; + + /** + * A popup is an element that has been displayed at a particular + * location within the page. + * @constructor + * @memberof platform/commonUI/general + * @param element the jqLite-wrapped element + * @param {object} styles an object containing key-value pairs + * of styles used to position the element. + */ + function Popup(element, styles) { + this.styles = styles; + this.element = element; + + element.css(styles); + } + + /** + * Stop showing this popup. + */ + Popup.prototype.dismiss = function () { + this.element.remove(); + }; + + /** + * Check if this popup is positioned such that it appears to the + * left of its original location. + * @returns {boolean} true if the popup goes left + */ + Popup.prototype.goesLeft = function () { + return !this.styles.left; + }; + + /** + * Check if this popup is positioned such that it appears to the + * right of its original location. + * @returns {boolean} true if the popup goes right + */ + Popup.prototype.goesRight = function () { + return !this.styles.right; + }; + + /** + * Check if this popup is positioned such that it appears above + * its original location. + * @returns {boolean} true if the popup goes up + */ + Popup.prototype.goesUp = function () { + return !this.styles.top; + }; + + /** + * Check if this popup is positioned such that it appears below + * its original location. + * @returns {boolean} true if the popup goes down + */ + Popup.prototype.goesDown = function () { + return !this.styles.bottom; + }; + + return Popup; + } +); diff --git a/platform/commonUI/general/src/services/PopupService.js b/platform/commonUI/general/src/services/PopupService.js index 3c655c035b..a7680d3dbd 100644 --- a/platform/commonUI/general/src/services/PopupService.js +++ b/platform/commonUI/general/src/services/PopupService.js @@ -22,7 +22,8 @@ /*global define*/ define( - function () { + ['./Popup'], + function (Popup) { "use strict"; /** @@ -73,14 +74,14 @@ define( * relative to the right or bottom of the window. * @param {PopupOptions} [options] additional options to control * positioning of the popup - * @returns {Function} a function that may be invoked to - * dismiss the info bubble + * @returns {platform/commonUI/general.Popup} the popup */ PopupService.prototype.display = function (element, position, options) { var $document = this.$document, $window = this.$window, body = $document.find('body'), winDim = [ $window.innerWidth, $window.innerHeight ], + styles = { position: 'absolute' }, margin, offset, bubble; @@ -107,32 +108,23 @@ define( position = position.map(adjustNegatives); - // Position the element - element.css('position', 'absolute'); - if (position[0] > margin[0]) { - element.css('right', (winDim[0] - position[0] + offset[0]) + 'px'); - applyClassOption('left'); + styles.right = (winDim[0] - position[0] + offset[0]) + 'px'; } else { - element.css('left', position[0] + offset[0] + 'px'); - applyClassOption('right'); + styles.left = (position[0] + offset[0]) + 'px'; } if (position[1] > margin[1]) { - element.css('bottom', (winDim[1] - position[1] + offset[1]) + 'px'); - applyClassOption('up'); + styles.bottom = (winDim[1] - position[1] + offset[1]) + 'px'; } else { - element.css('top', position[1] + offset[1] + 'px'); - applyClassOption('down'); + styles.top = (position[1] + offset[1]) + 'px'; } // Add the menu to the body body.append(element); // Return a function to dismiss the bubble - return function () { - element.remove(); - }; + return new Popup(element, styles); }; return PopupService; diff --git a/platform/commonUI/inspect/src/InfoConstants.js b/platform/commonUI/inspect/src/InfoConstants.js index 4475f58439..facf7ef103 100644 --- a/platform/commonUI/inspect/src/InfoConstants.js +++ b/platform/commonUI/inspect/src/InfoConstants.js @@ -32,17 +32,14 @@ define({ "bubble-title=\"{{bubbleTitle}}\" " + "bubble-layout=\"{{bubbleLayout}}\" " + "class=\"bubble-container\">" + - "" + + "" + "" + "", // Options and classes for bubble BUBBLE_OPTIONS: { offsetX: 0, - offsetY: -26, - leftClass: 'arw-left', - rightClass: 'arw-right', - topClass: 'arw-top', - bottomClss: 'arw-btm' + offsetY: -26 }, BUBBLE_MOBILE_POSITION: [ 0, -25 ], // Max width and margins allowed for bubbles; defined in /platform/commonUI/general/res/sass/_constants.scss diff --git a/platform/commonUI/inspect/src/services/InfoService.js b/platform/commonUI/inspect/src/services/InfoService.js index cd3fa4087b..e490171d08 100644 --- a/platform/commonUI/inspect/src/services/InfoService.js +++ b/platform/commonUI/inspect/src/services/InfoService.js @@ -60,7 +60,7 @@ define( bubbleSpaceLR = InfoConstants.BUBBLE_MARGIN_LR + InfoConstants.BUBBLE_MAX_WIDTH, options, - dismissPopup, + popup, bubble; // Pass model & container parameters into the scope @@ -81,12 +81,18 @@ define( options = {}; } - dismissPopup = - this.popupService.display(bubble, position, options); + popup = this.popupService.display(bubble, position, options); + + // Style the bubble according to how it was positioned + if (popup.goesLeft()) { + scope.bubbleLayout = 'arw-right'; + } else if (popup.goesRight()) { + scope.bubbleLayout = 'arw-left'; + } // Return a function to dismiss the info bubble return function dismiss() { - dismissPopup(); + popup.dismiss(); scope.$destroy(); }; }; From bebe53820fbadb102d63638d307c7bd957e13f69 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 09:36:27 -0700 Subject: [PATCH 13/28] [Info Service] Choose arrow direction --- platform/commonUI/inspect/src/services/InfoService.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/platform/commonUI/inspect/src/services/InfoService.js b/platform/commonUI/inspect/src/services/InfoService.js index e490171d08..e960918a3f 100644 --- a/platform/commonUI/inspect/src/services/InfoService.js +++ b/platform/commonUI/inspect/src/services/InfoService.js @@ -84,11 +84,10 @@ define( popup = this.popupService.display(bubble, position, options); // Style the bubble according to how it was positioned - if (popup.goesLeft()) { - scope.bubbleLayout = 'arw-right'; - } else if (popup.goesRight()) { - scope.bubbleLayout = 'arw-left'; - } + scope.bubbleLayout = [ + popup.goesLeft() ? 'arw-right' : 'arw-left', + popup.goesUp() ? 'arw-btm' : 'arw-top' + ].join(' '); // Return a function to dismiss the info bubble return function dismiss() { From 73dc16d398b134943392abffbe60aeb45964513f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 10:21:42 -0700 Subject: [PATCH 14/28] [Info Service] Render info bubble after positioning Render info bubble after positioning with the popupService, to apply arrow classes appropriately. --- .../commonUI/inspect/src/InfoConstants.js | 3 ++- .../inspect/src/services/InfoService.js | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/platform/commonUI/inspect/src/InfoConstants.js b/platform/commonUI/inspect/src/InfoConstants.js index facf7ef103..33a0865dd9 100644 --- a/platform/commonUI/inspect/src/InfoConstants.js +++ b/platform/commonUI/inspect/src/InfoConstants.js @@ -42,7 +42,8 @@ define({ offsetY: -26 }, BUBBLE_MOBILE_POSITION: [ 0, -25 ], - // Max width and margins allowed for bubbles; defined in /platform/commonUI/general/res/sass/_constants.scss + // Max width and margins allowed for bubbles; + // defined in /platform/commonUI/general/res/sass/_constants.scss BUBBLE_MARGIN_LR: 10, BUBBLE_MAX_WIDTH: 300 }); diff --git a/platform/commonUI/inspect/src/services/InfoService.js b/platform/commonUI/inspect/src/services/InfoService.js index e960918a3f..eb929027ac 100644 --- a/platform/commonUI/inspect/src/services/InfoService.js +++ b/platform/commonUI/inspect/src/services/InfoService.js @@ -57,20 +57,13 @@ define( var $compile = this.$compile, $rootScope = this.$rootScope, scope = $rootScope.$new(), + span = $compile('')(scope), bubbleSpaceLR = InfoConstants.BUBBLE_MARGIN_LR + InfoConstants.BUBBLE_MAX_WIDTH, options, popup, bubble; - // Pass model & container parameters into the scope - scope.bubbleModel = content; - scope.bubbleTemplate = templateKey; - scope.bubbleTitle = title; - - // Create the context menu - bubble = $compile(BUBBLE_TEMPLATE)(scope); - options = Object.create(OPTIONS); options.marginX = -bubbleSpaceLR; @@ -81,13 +74,23 @@ define( options = {}; } - popup = this.popupService.display(bubble, position, options); + popup = this.popupService.display(span, position, options); + // Pass model & container parameters into the scope + scope.bubbleModel = content; + scope.bubbleTemplate = templateKey; + scope.bubbleTitle = title; // Style the bubble according to how it was positioned scope.bubbleLayout = [ - popup.goesLeft() ? 'arw-right' : 'arw-left', - popup.goesUp() ? 'arw-btm' : 'arw-top' + popup.goesUp() ? 'arw-btm' : 'arw-top', + popup.goesLeft() ? 'arw-right' : 'arw-left' ].join(' '); + scope.bubbleLayout = 'arw-top arw-left'; + + // Create the info bubble, now that we know how to + // point the arrow... + bubble = $compile(BUBBLE_TEMPLATE)(scope); + span.append(bubble); // Return a function to dismiss the info bubble return function dismiss() { From 1ad0bf337cfa8131fdb5368e98c69d9efac1da49 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 10:48:41 -0700 Subject: [PATCH 15/28] [Common UI] Use popupService from mct-popup --- platform/commonUI/general/bundle.json | 2 +- .../general/src/directives/MCTPopup.js | 27 +++++-------------- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/platform/commonUI/general/bundle.json b/platform/commonUI/general/bundle.json index 8e367cc060..1aa0b1dfc1 100644 --- a/platform/commonUI/general/bundle.json +++ b/platform/commonUI/general/bundle.json @@ -133,7 +133,7 @@ { "key": "mctPopup", "implementation": "directives/MCTPopup.js", - "depends": [ "$window", "$document", "$compile", "$interval" ] + "depends": [ "$compile", "popupService" ] }, { "key": "mctScrollX", diff --git a/platform/commonUI/general/src/directives/MCTPopup.js b/platform/commonUI/general/src/directives/MCTPopup.js index aa411684ae..d5ced4129e 100644 --- a/platform/commonUI/general/src/directives/MCTPopup.js +++ b/platform/commonUI/general/src/directives/MCTPopup.js @@ -41,37 +41,22 @@ define( * * @constructor * @memberof platform/commonUI/general - * @param $window the window object (as injected by Angular) - * @param $document the jqLite-wrapped document * @param $compile Angular's $compile service + * @param {platform/commonUI/general.PopupService} popupService */ - function MCTPopup($window, $document, $compile) { + function MCTPopup($compile, popupService) { function link(scope, element, attrs, ctrl, transclude) { - var body = $document.find('body'), - popup = $compile(TEMPLATE)(scope), - winDim = [$window.innerWidth, $window.innerHeight], + var div = $compile(TEMPLATE)(scope), rect = element.parent()[0].getBoundingClientRect(), position = [ rect.left, rect.top ], - isLeft = position[0] <= (winDim[0] / 2), - isTop = position[1] <= (winDim[1] / 2); - - popup.css('position', 'absolute'); - popup.css( - isLeft ? 'left' : 'right', - (isLeft ? position[0] : (winDim[0] - position[0])) + 'px' - ); - popup.css( - isTop ? 'top' : 'bottom', - (isTop ? position[1] : (winDim[1] - position[1])) + 'px' - ); - body.append(popup); + popup = popupService.display(div, position); transclude(function (clone) { - popup.append(clone); + div.append(clone); }); scope.$on('$destroy', function () { - popup.remove(); + popup.dismiss(); }); } From 445f22ccb0868409dab6d684d0277858c42c4026 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 11:01:49 -0700 Subject: [PATCH 16/28] [Context Menu] Use popupService to display menus --- platform/representation/bundle.json | 8 ++- .../src/actions/ContextMenuAction.js | 54 ++++++++++--------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/platform/representation/bundle.json b/platform/representation/bundle.json index 44656b0f4c..331856a9a8 100644 --- a/platform/representation/bundle.json +++ b/platform/representation/bundle.json @@ -54,7 +54,13 @@ { "key": "menu", "implementation": "actions/ContextMenuAction.js", - "depends": [ "$compile", "$document", "$window", "$rootScope", "agentService" ] + "depends": [ + "$compile", + "$document", + "$rootScope", + "popupService", + "agentService" + ] } ] } diff --git a/platform/representation/src/actions/ContextMenuAction.js b/platform/representation/src/actions/ContextMenuAction.js index 56e5fa33f9..cbb6fcf8b3 100644 --- a/platform/representation/src/actions/ContextMenuAction.js +++ b/platform/representation/src/actions/ContextMenuAction.js @@ -43,40 +43,50 @@ define( * @constructor * @param $compile Angular's $compile service * @param $document the current document - * @param $window the active window * @param $rootScope Angular's root scope - * @param actionContexr the context in which the action + * @param {platform/commonUI/general.PopupService} popupService + * @param actionContext the context in which the action * should be performed * @implements {Action} */ - function ContextMenuAction($compile, $document, $window, $rootScope, agentService, actionContext) { + function ContextMenuAction( + $compile, + $document, + $rootScope, + popupService, + agentService, + actionContext + ) { this.$compile = $compile; this.agentService = agentService; this.actionContext = actionContext; + this.popupService = popupService; this.getDocument = function () { return $document; }; - this.getWindow = function () { return $window; }; this.getRootScope = function () { return $rootScope; }; } ContextMenuAction.prototype.perform = function () { var $compile = this.$compile, $document = this.getDocument(), - $window = this.getWindow(), $rootScope = this.getRootScope(), actionContext = this.actionContext, - winDim = [$window.innerWidth, $window.innerHeight], - eventCoors = [actionContext.event.pageX, actionContext.event.pageY], + eventCoords = [ + actionContext.event.pageX, + actionContext.event.pageY + ], menuDim = GestureConstants.MCT_MENU_DIMENSIONS, body = $document.find('body'), scope = $rootScope.$new(), - goLeft = eventCoors[0] + menuDim[0] > winDim[0], - goUp = eventCoors[1] + menuDim[1] > winDim[1], - initiatingEvent = this.agentService.isMobile() ? 'touchstart' : 'mousedown', - menu; + initiatingEvent = this.agentService.isMobile() ? + 'touchstart' : 'mousedown', + menu, + popup; // Remove the context menu function dismiss() { - menu.remove(); + popup.dismiss(); + popup = undefined; + scope.$destroy(); body.off("mousedown", dismiss); dismissExistingMenu = undefined; } @@ -91,21 +101,17 @@ define( // Set up the scope, including menu positioning scope.domainObject = actionContext.domainObject; - scope.menuStyle = {}; - scope.menuStyle[goLeft ? "right" : "left"] = - (goLeft ? (winDim[0] - eventCoors[0]) : eventCoors[0]) + 'px'; - scope.menuStyle[goUp ? "bottom" : "top"] = - (goUp ? (winDim[1] - eventCoors[1]) : eventCoors[1]) + 'px'; - scope.menuClass = { - "go-left": goLeft, - "go-up": goUp, - "context-menu-holder": true - }; + scope.menuClass = { "context-menu-holder": true }; // Create the context menu menu = $compile(MENU_TEMPLATE)(scope); - // Add the menu to the body - body.append(menu); + popup = this.popupService.display(menu, eventCoords, { + marginX: -menuDim[0], + marginY: -menuDim[1] + }); + + scope.menuClass['go-left'] = popup.goesLeft(); + scope.menuClass['go-up'] = popup.goesUp(); // Stop propagation so that clicks or touches on the menu do not close the menu menu.on(initiatingEvent, function (event) { From 13095b4135d889c7f617f844b79afdee2967ca0f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 14:14:34 -0700 Subject: [PATCH 17/28] [Context Menu] Update specs Update specs to reflect refactoring-out of popup elements performed in the context of adding time conductor, WTD-1515. --- .../src/actions/ContextMenuAction.js | 6 +- .../test/actions/ContextMenuActionSpec.js | 75 ++++++++++--------- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/platform/representation/src/actions/ContextMenuAction.js b/platform/representation/src/actions/ContextMenuAction.js index cbb6fcf8b3..82e11713f9 100644 --- a/platform/representation/src/actions/ContextMenuAction.js +++ b/platform/representation/src/actions/ContextMenuAction.js @@ -84,8 +84,10 @@ define( // Remove the context menu function dismiss() { - popup.dismiss(); - popup = undefined; + if (popup) { + popup.dismiss(); + popup = undefined; + } scope.$destroy(); body.off("mousedown", dismiss); dismissExistingMenu = undefined; diff --git a/platform/representation/test/actions/ContextMenuActionSpec.js b/platform/representation/test/actions/ContextMenuActionSpec.js index 233d6c6bf0..ba24076fbb 100644 --- a/platform/representation/test/actions/ContextMenuActionSpec.js +++ b/platform/representation/test/actions/ContextMenuActionSpec.js @@ -41,13 +41,14 @@ define( mockMenu, mockDocument, mockBody, - mockWindow, + mockPopupService, mockRootScope, mockAgentService, mockScope, mockElement, mockDomainObject, mockEvent, + mockPopup, mockActionContext, action; @@ -57,36 +58,47 @@ define( mockMenu = jasmine.createSpyObj("menu", JQLITE_FUNCTIONS); mockDocument = jasmine.createSpyObj("$document", JQLITE_FUNCTIONS); mockBody = jasmine.createSpyObj("body", JQLITE_FUNCTIONS); - mockWindow = { innerWidth: MENU_DIMENSIONS[0] * 4, innerHeight: MENU_DIMENSIONS[1] * 4 }; + mockPopupService = + jasmine.createSpyObj("popupService", ["display"]); + mockPopup = jasmine.createSpyObj("popup", [ + "dismiss", + "goesLeft", + "goesUp" + ]); mockRootScope = jasmine.createSpyObj("$rootScope", ["$new"]); mockAgentService = jasmine.createSpyObj("agentService", ["isMobile"]); - mockScope = {}; + mockScope = jasmine.createSpyObj("scope", ["$destroy"]); mockElement = jasmine.createSpyObj("element", JQLITE_FUNCTIONS); mockDomainObject = jasmine.createSpyObj("domainObject", DOMAIN_OBJECT_METHODS); mockEvent = jasmine.createSpyObj("event", ["preventDefault", "stopPropagation"]); - mockEvent.pageX = 0; - mockEvent.pageY = 0; + mockEvent.pageX = 123; + mockEvent.pageY = 321; mockCompile.andReturn(mockCompiledTemplate); mockCompiledTemplate.andReturn(mockMenu); mockDocument.find.andReturn(mockBody); mockRootScope.$new.andReturn(mockScope); + mockPopupService.display.andReturn(mockPopup); mockActionContext = {key: 'menu', domainObject: mockDomainObject, event: mockEvent}; action = new ContextMenuAction( mockCompile, mockDocument, - mockWindow, mockRootScope, + mockPopupService, mockAgentService, mockActionContext ); }); - it(" adds a menu to the DOM when perform is called", function () { + it("displays a popup when performed", function () { action.perform(); - expect(mockBody.append).toHaveBeenCalledWith(mockMenu); + expect(mockPopupService.display).toHaveBeenCalledWith( + mockMenu, + [ mockEvent.pageX, mockEvent.pageY ], + jasmine.any(Object) + ); }); it("prevents the default context menu behavior", function () { @@ -94,29 +106,22 @@ define( expect(mockEvent.preventDefault).toHaveBeenCalled(); }); - it("positions menus where clicked", function () { - mockEvent.pageX = 10; - mockEvent.pageY = 5; - action.perform(); - expect(mockScope.menuStyle.left).toEqual("10px"); - expect(mockScope.menuStyle.top).toEqual("5px"); - expect(mockScope.menuStyle.right).toBeUndefined(); - expect(mockScope.menuStyle.bottom).toBeUndefined(); - expect(mockScope.menuClass['go-up']).toBeFalsy(); - expect(mockScope.menuClass['go-left']).toBeFalsy(); + it("adds classes to menus based on position", function () { + var booleans = [ false, true ]; + + booleans.forEach(function (goLeft) { + booleans.forEach(function (goUp) { + mockPopup.goesLeft.andReturn(goLeft); + mockPopup.goesUp.andReturn(goUp); + action.perform(); + expect(!!mockScope.menuClass['go-up']) + .toEqual(goUp); + expect(!!mockScope.menuClass['go-left']) + .toEqual(goLeft); + }); + }); }); - it("repositions menus near the screen edge", function () { - mockEvent.pageX = mockWindow.innerWidth - 10; - mockEvent.pageY = mockWindow.innerHeight - 5; - action.perform(); - expect(mockScope.menuStyle.right).toEqual("10px"); - expect(mockScope.menuStyle.bottom).toEqual("5px"); - expect(mockScope.menuStyle.left).toBeUndefined(); - expect(mockScope.menuStyle.top).toBeUndefined(); - expect(mockScope.menuClass['go-up']).toBeTruthy(); - expect(mockScope.menuClass['go-left']).toBeTruthy(); - }); it("removes a menu when body is clicked", function () { // Show the menu @@ -133,7 +138,7 @@ define( }); // Menu should have been removed - expect(mockMenu.remove).toHaveBeenCalled(); + expect(mockPopup.dismiss).toHaveBeenCalled(); // Listener should have been detached from body expect(mockBody.off).toHaveBeenCalledWith( @@ -149,7 +154,7 @@ define( // Verify precondition expect(mockMenu.remove).not.toHaveBeenCalled(); - // Find and fire body's mousedown listener + // Find and fire menu's click listener mockMenu.on.calls.forEach(function (call) { if (call.args[0] === 'click') { call.args[1](); @@ -157,7 +162,7 @@ define( }); // Menu should have been removed - expect(mockMenu.remove).toHaveBeenCalled(); + expect(mockPopup.dismiss).toHaveBeenCalled(); }); it("keeps a menu when menu is clicked", function () { @@ -171,7 +176,7 @@ define( }); // Menu should have been removed - expect(mockMenu.remove).not.toHaveBeenCalled(); + expect(mockPopup.dismiss).not.toHaveBeenCalled(); // Listener should have been detached from body expect(mockBody.off).not.toHaveBeenCalled(); @@ -182,8 +187,8 @@ define( action = new ContextMenuAction( mockCompile, mockDocument, - mockWindow, mockRootScope, + mockPopupService, mockAgentService, mockActionContext ); @@ -194,6 +199,8 @@ define( call.args[1](mockEvent); } }); + + expect(mockPopup.dismiss).not.toHaveBeenCalled(); }); }); } From c4aff953414f3f3c9a1964fc16d753f1a279bac2 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 14:15:31 -0700 Subject: [PATCH 18/28] [Common UI] Add empty specs for popupService --- .../general/test/services/PopupServiceSpec.js | 32 +++++++++++++++++ .../general/test/services/PopupSpec.js | 35 +++++++++++++++++++ platform/commonUI/general/test/suite.json | 2 ++ 3 files changed, 69 insertions(+) create mode 100644 platform/commonUI/general/test/services/PopupServiceSpec.js create mode 100644 platform/commonUI/general/test/services/PopupSpec.js diff --git a/platform/commonUI/general/test/services/PopupServiceSpec.js b/platform/commonUI/general/test/services/PopupServiceSpec.js new file mode 100644 index 0000000000..1183845810 --- /dev/null +++ b/platform/commonUI/general/test/services/PopupServiceSpec.js @@ -0,0 +1,32 @@ +/***************************************************************************** + * 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/services/PopupService"], + function (PopupService) { + 'use strict'; + + + } +); diff --git a/platform/commonUI/general/test/services/PopupSpec.js b/platform/commonUI/general/test/services/PopupSpec.js new file mode 100644 index 0000000000..3b6db0394e --- /dev/null +++ b/platform/commonUI/general/test/services/PopupSpec.js @@ -0,0 +1,35 @@ +/***************************************************************************** + * 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/services/Popup"], + function (Popup) { + 'use strict'; + + describe("Popup", function () { + + }); + + } +); diff --git a/platform/commonUI/general/test/suite.json b/platform/commonUI/general/test/suite.json index 1427d70f3a..0d19fbb9e4 100644 --- a/platform/commonUI/general/test/suite.json +++ b/platform/commonUI/general/test/suite.json @@ -17,6 +17,8 @@ "directives/MCTPopup", "directives/MCTResize", "directives/MCTScroll", + "services/Popup", + "services/PopupService", "services/UrlService", "StyleSheetLoader" ] From 553b17fafe91b295d4416e9ff8b812b58ca0d214 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 14:36:04 -0700 Subject: [PATCH 19/28] [Common UI] Update mct-popup spec ...to reflect usage of popupService. --- .../general/test/directives/MCTPopupSpec.js | 55 +++++++++++++++---- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/platform/commonUI/general/test/directives/MCTPopupSpec.js b/platform/commonUI/general/test/directives/MCTPopupSpec.js index 94fd3f0d0e..2cd6598180 100644 --- a/platform/commonUI/general/test/directives/MCTPopupSpec.js +++ b/platform/commonUI/general/test/directives/MCTPopupSpec.js @@ -29,15 +29,16 @@ define( var JQLITE_METHODS = [ "on", "off", "find", "parent", "css", "append" ]; describe("The mct-popup directive", function () { - var testWindow, - mockDocument, - mockCompile, + var mockCompile, + mockPopupService, + mockPopup, mockScope, mockElement, testAttrs, mockBody, mockTransclude, mockParentEl, + mockNewElement, testRect, mctPopup; @@ -50,12 +51,12 @@ define( } beforeEach(function () { - testWindow = - { innerWidth: 600, innerHeight: 300 }; - mockDocument = - jasmine.createSpyObj("$document", JQLITE_METHODS); mockCompile = jasmine.createSpy("$compile"); + mockPopupService = + jasmine.createSpyObj("popupService", ["display"]); + mockPopup = + jasmine.createSpyObj("popup", ["dismiss"]); mockScope = jasmine.createSpyObj("$scope", [ "$eval", "$apply", "$on" ]); mockElement = @@ -66,6 +67,8 @@ define( jasmine.createSpy("transclude"); mockParentEl = jasmine.createSpyObj("parent", ["getBoundingClientRect"]); + mockNewElement = + jasmine.createSpyObj("newElement", JQLITE_METHODS); testAttrs = { mctClickElsewhere: "some Angular expression" @@ -77,15 +80,17 @@ define( height: 75 }; - mockDocument.find.andReturn(mockBody); - mockCompile.andReturn(jasmine.createSpy()); - mockCompile().andCallFake(function () { - return jasmine.createSpyObj("newElement", JQLITE_METHODS); + mockCompile.andCallFake(function () { + var mockFn = jasmine.createSpy(); + mockFn.andReturn(mockNewElement); + return mockFn; }); mockElement.parent.andReturn([mockParentEl]); mockParentEl.getBoundingClientRect.andReturn(testRect); + mockPopupService.display.andReturn(mockPopup); + + mctPopup = new MCTPopup(mockCompile, mockPopupService); - mctPopup = new MCTPopup(testWindow, mockDocument, mockCompile); mctPopup.link( mockScope, mockElement, @@ -99,6 +104,32 @@ define( expect(mctPopup.restrict).toEqual("E"); }); + describe("creates an element which", function () { + it("displays as a popup", function () { + expect(mockPopupService.display).toHaveBeenCalledWith( + mockNewElement, + [ testRect.left, testRect.top ] + ); + }); + + it("displays transcluded content", function () { + var mockClone = + jasmine.createSpyObj('clone', JQLITE_METHODS); + mockTransclude.mostRecentCall.args[0](mockClone); + expect(mockNewElement.append) + .toHaveBeenCalledWith(mockClone); + }); + + it("is removed when its containing scope is destroyed", function () { + expect(mockPopup.dismiss).not.toHaveBeenCalled(); + mockScope.$on.calls.forEach(function (call) { + if (call.args[0] === '$destroy') { + call.args[1](); + } + }); + expect(mockPopup.dismiss).toHaveBeenCalled(); + }); + }); }); } From 99048a4ee339d9ad72ba6fa6aedbc47de810cfa5 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 14:49:27 -0700 Subject: [PATCH 20/28] [Info Service] Update spec ...to reflect usage of popupService. --- .../inspect/test/services/InfoServiceSpec.js | 140 +++++++----------- 1 file changed, 54 insertions(+), 86 deletions(-) diff --git a/platform/commonUI/inspect/test/services/InfoServiceSpec.js b/platform/commonUI/inspect/test/services/InfoServiceSpec.js index e878afb268..f55d72d04f 100644 --- a/platform/commonUI/inspect/test/services/InfoServiceSpec.js +++ b/platform/commonUI/inspect/test/services/InfoServiceSpec.js @@ -28,117 +28,85 @@ define( describe("The info service", function () { var mockCompile, - mockDocument, - testWindow, mockRootScope, + mockPopupService, mockAgentService, - mockCompiledTemplate, - testScope, - mockBody, - mockElement, + mockScope, + mockElements, + mockPopup, service; beforeEach(function () { mockCompile = jasmine.createSpy('$compile'); - mockDocument = jasmine.createSpyObj('$document', ['find']); - testWindow = { innerWidth: 1000, innerHeight: 100 }; mockRootScope = jasmine.createSpyObj('$rootScope', ['$new']); mockAgentService = jasmine.createSpyObj('agentService', ['isMobile', 'isPhone']); - mockCompiledTemplate = jasmine.createSpy('template'); - testScope = {}; - mockBody = jasmine.createSpyObj('body', ['append']); - mockElement = jasmine.createSpyObj('element', ['css', 'remove']); + mockPopupService = jasmine.createSpyObj( + 'popupService', + ['display'] + ); + mockPopup = jasmine.createSpyObj('popup', [ + 'dismiss', + 'goesLeft', + 'goesRight', + 'goesUp', + 'goesDown' + ]); - mockDocument.find.andCallFake(function (tag) { - return tag === 'body' ? mockBody : undefined; + mockScope = jasmine.createSpyObj("scope", ["$destroy"]); + mockElements = []; + + mockPopupService.display.andReturn(mockPopup); + mockCompile.andCallFake(function () { + var mockCompiledTemplate = jasmine.createSpy('template'), + mockElement = jasmine.createSpyObj('element', [ + 'css', + 'remove', + 'append' + ]); + mockCompiledTemplate.andReturn(mockElement); + mockElements.push(mockElement); + return mockCompiledTemplate; }); - mockCompile.andReturn(mockCompiledTemplate); - mockCompiledTemplate.andReturn(mockElement); - mockRootScope.$new.andReturn(testScope); + mockRootScope.$new.andReturn(mockScope); service = new InfoService( mockCompile, - mockDocument, - testWindow, mockRootScope, + mockPopupService, mockAgentService ); }); - it("creates elements and appends them to the body to display", function () { - service.display('', '', {}, [0, 0]); - expect(mockBody.append).toHaveBeenCalledWith(mockElement); + it("creates elements and displays them as popups", function () { + service.display('', '', {}, [123, 456]); + expect(mockPopupService.display).toHaveBeenCalledWith( + mockElements[0], + [ 123, 456 ], + jasmine.any(Object) + ); }); it("provides a function to remove displayed info bubbles", function () { var fn = service.display('', '', {}, [0, 0]); - expect(mockElement.remove).not.toHaveBeenCalled(); + expect(mockPopup.dismiss).not.toHaveBeenCalled(); fn(); - expect(mockElement.remove).toHaveBeenCalled(); + expect(mockPopup.dismiss).toHaveBeenCalled(); }); - describe("depending on mouse position", function () { - // Positioning should vary based on quadrant in window, - // which is 1000 x 100 in this test case. - it("displays from the top-left in the top-left quadrant", function () { - service.display('', '', {}, [250, 25]); - expect(mockElement.css).toHaveBeenCalledWith( - 'left', - (250 + InfoConstants.BUBBLE_OFFSET[0]) + 'px' - ); - expect(mockElement.css).toHaveBeenCalledWith( - 'top', - (25 + InfoConstants.BUBBLE_OFFSET[1]) + 'px' - ); - }); - - it("displays from the top-right in the top-right quadrant", function () { - service.display('', '', {}, [700, 25]); - expect(mockElement.css).toHaveBeenCalledWith( - 'right', - (300 + InfoConstants.BUBBLE_OFFSET[0]) + 'px' - ); - expect(mockElement.css).toHaveBeenCalledWith( - 'top', - (25 + InfoConstants.BUBBLE_OFFSET[1]) + 'px' - ); - }); - - it("displays from the bottom-left in the bottom-left quadrant", function () { - service.display('', '', {}, [250, 70]); - expect(mockElement.css).toHaveBeenCalledWith( - 'left', - (250 + InfoConstants.BUBBLE_OFFSET[0]) + 'px' - ); - expect(mockElement.css).toHaveBeenCalledWith( - 'bottom', - (30 + InfoConstants.BUBBLE_OFFSET[1]) + 'px' - ); - }); - - it("displays from the bottom-right in the bottom-right quadrant", function () { - service.display('', '', {}, [800, 60]); - expect(mockElement.css).toHaveBeenCalledWith( - 'right', - (200 + InfoConstants.BUBBLE_OFFSET[0]) + 'px' - ); - expect(mockElement.css).toHaveBeenCalledWith( - 'bottom', - (40 + InfoConstants.BUBBLE_OFFSET[1]) + 'px' - ); - }); - - it("when on phone device, positioning is always on bottom", function () { - mockAgentService.isPhone.andReturn(true); - service = new InfoService( - mockCompile, - mockDocument, - testWindow, - mockRootScope, - mockAgentService - ); - service.display('', '', {}, [0, 0]); - }); + it("when on phone device, positions at bottom", function () { + mockAgentService.isPhone.andReturn(true); + service = new InfoService( + mockCompile, + mockRootScope, + mockPopupService, + mockAgentService + ); + service.display('', '', {}, [123, 456]); + expect(mockPopupService.display).toHaveBeenCalledWith( + mockElements[0], + [ 0, -25 ], + jasmine.any(Object) + ); }); }); From a1d1261179a6bad01a4df8bd79a482afadc78829 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 14:58:20 -0700 Subject: [PATCH 21/28] [Common UI] Test Popup --- .../general/test/services/PopupSpec.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/platform/commonUI/general/test/services/PopupSpec.js b/platform/commonUI/general/test/services/PopupSpec.js index 3b6db0394e..84d63953a2 100644 --- a/platform/commonUI/general/test/services/PopupSpec.js +++ b/platform/commonUI/general/test/services/PopupSpec.js @@ -28,6 +28,45 @@ define( 'use strict'; describe("Popup", function () { + var mockElement, + testStyles, + popup; + + beforeEach(function () { + mockElement = + jasmine.createSpyObj('element', [ 'css', 'remove' ]); + testStyles = { left: '12px', top: '14px' }; + popup = new Popup(mockElement, testStyles); + }); + + it("applies CSS styles when instantiated", function () { + expect(mockElement.css) + .toHaveBeenCalledWith(testStyles); + }); + + it("reports the orientation of the popup", function () { + var otherStyles = { + right: '12px', + bottom: '14px' + }, + otherPopup = new Popup(mockElement, otherStyles); + + expect(popup.goesLeft()).toBeFalsy(); + expect(popup.goesRight()).toBeTruthy(); + expect(popup.goesUp()).toBeFalsy(); + expect(popup.goesDown()).toBeTruthy(); + + expect(otherPopup.goesLeft()).toBeTruthy(); + expect(otherPopup.goesRight()).toBeFalsy(); + expect(otherPopup.goesUp()).toBeTruthy(); + expect(otherPopup.goesDown()).toBeFalsy(); + }); + + it("removes elements when dismissed", function () { + expect(mockElement.remove).not.toHaveBeenCalled(); + popup.dismiss(); + expect(mockElement.remove).toHaveBeenCalled(); + }); }); From 1ca2b769d9d629204eaa41a5882a06683b6c6d45 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 15:44:36 -0700 Subject: [PATCH 22/28] [Common UI] Test popupService --- .../general/src/services/PopupService.js | 6 -- .../general/test/services/PopupServiceSpec.js | 66 +++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/general/src/services/PopupService.js b/platform/commonUI/general/src/services/PopupService.js index a7680d3dbd..f834609f2c 100644 --- a/platform/commonUI/general/src/services/PopupService.js +++ b/platform/commonUI/general/src/services/PopupService.js @@ -86,12 +86,6 @@ define( offset, bubble; - function applyClassOption(direction) { - if (options[direction + 'Class']) { - element.addClass(options[direction + 'Class']); - } - } - function adjustNegatives(value, index) { return value < 0 ? (value + winDim[index]) : value; } diff --git a/platform/commonUI/general/test/services/PopupServiceSpec.js b/platform/commonUI/general/test/services/PopupServiceSpec.js index 1183845810..741d23bd37 100644 --- a/platform/commonUI/general/test/services/PopupServiceSpec.js +++ b/platform/commonUI/general/test/services/PopupServiceSpec.js @@ -27,6 +27,72 @@ define( function (PopupService) { 'use strict'; + describe("PopupService", function () { + var mockDocument, + testWindow, + mockBody, + mockElement, + popupService; + beforeEach(function () { + mockDocument = jasmine.createSpyObj('$document', [ 'find' ]); + testWindow = { innerWidth: 1000, innerHeight: 800 }; + mockBody = jasmine.createSpyObj('body', [ 'append' ]); + mockElement = jasmine.createSpyObj('element', [ + 'css', + 'remove' + ]); + + mockDocument.find.andCallFake(function (query) { + return query === 'body' && mockBody; + }); + + popupService = new PopupService(mockDocument, testWindow); + }); + + it("adds elements to the body of the document", function () { + popupService.display(mockElement, [ 0, 0 ]); + expect(mockBody.append).toHaveBeenCalledWith(mockElement); + }); + + describe("when positioned in appropriate quadrants", function () { + it("orients elements relative to the top-left", function () { + popupService.display(mockElement, [ 25, 50 ]); + expect(mockElement.css).toHaveBeenCalledWith({ + position: 'absolute', + left: '25px', + top: '50px' + }); + }); + + it("orients elements relative to the top-right", function () { + popupService.display(mockElement, [ 800, 50 ]); + expect(mockElement.css).toHaveBeenCalledWith({ + position: 'absolute', + right: '200px', + top: '50px' + }); + }); + + it("orients elements relative to the bottom-right", function () { + popupService.display(mockElement, [ 800, 650 ]); + expect(mockElement.css).toHaveBeenCalledWith({ + position: 'absolute', + right: '200px', + bottom: '150px' + }); + }); + + it("orients elements relative to the bottom-left", function () { + popupService.display(mockElement, [ 120, 650 ]); + expect(mockElement.css).toHaveBeenCalledWith({ + position: 'absolute', + left: '120px', + bottom: '150px' + }); + }); + }); + + }); } ); From dd838160353fe6bf0b8f3013214104b7f3733803 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 16:05:23 -0700 Subject: [PATCH 23/28] [Time Conductor] Remove queryStart, queryEnd ...per feedback from code review, nasa/openmctweb#104 --- .../conductor/src/ConductorRepresenter.js | 35 +++++------------- .../features/conductor/src/TimeConductor.js | 36 +++---------------- .../test/ConductorRepresenterSpec.js | 12 ++----- .../conductor/test/ConductorServiceSpec.js | 6 ++-- .../conductor/test/TimeConductorSpec.js | 6 ---- 5 files changed, 19 insertions(+), 76 deletions(-) diff --git a/platform/features/conductor/src/ConductorRepresenter.js b/platform/features/conductor/src/ConductorRepresenter.js index b1b35477b6..212c2e6a71 100644 --- a/platform/features/conductor/src/ConductorRepresenter.js +++ b/platform/features/conductor/src/ConductorRepresenter.js @@ -60,40 +60,24 @@ define( this.$compile = $compile; } - // Combine start/end times into a single object - function bounds(start, end) { - return { start: start, end: end }; - } - // Update the time conductor from the scope function wireScope(conductor, conductorScope, repScope) { - function updateConductorOuter() { - conductor.queryStart(conductorScope.conductor.outer.start); - conductor.queryEnd(conductorScope.conductor.outer.end); - repScope.$broadcast( - 'telemetry:query:bounds', - bounds(conductor.queryStart(), conductor.queryEnd()) - ); + // Combine start/end times into a single object + function bounds(start, end) { + return { + start: conductor.displayStart(), + end: conductor.displayEnd() + }; } function updateConductorInner() { conductor.displayStart(conductorScope.conductor.inner.start); conductor.displayEnd(conductorScope.conductor.inner.end); - repScope.$broadcast( - 'telemetry:display:bounds', - bounds(conductor.displayStart(), conductor.displayEnd()) - ); + repScope.$broadcast('telemetry:display:bounds', bounds()); } - conductorScope.conductor = { - outer: bounds(conductor.queryStart(), conductor.queryEnd()), - inner: bounds(conductor.displayStart(), conductor.displayEnd()) - }; + conductorScope.conductor = { outer: bounds(), inner: bounds() }; - conductorScope - .$watch('conductor.outer.start', updateConductorOuter); - conductorScope - .$watch('conductor.outer.end', updateConductorOuter); conductorScope .$watch('conductor.inner.start', updateConductorInner); conductorScope @@ -103,8 +87,7 @@ define( } ConductorRepresenter.prototype.conductorScope = function (s) { - return (this.cScope = arguments.length > 0 ? - s : this.cScope); + return (this.cScope = arguments.length > 0 ? s : this.cScope); }; // Handle a specific representation of a specific domain object diff --git a/platform/features/conductor/src/TimeConductor.js b/platform/features/conductor/src/TimeConductor.js index fcf8dbae04..394d9b01bb 100644 --- a/platform/features/conductor/src/TimeConductor.js +++ b/platform/features/conductor/src/TimeConductor.js @@ -41,35 +41,9 @@ define( * @param {number} end the initial end time */ function TimeConductor(start, end) { - this.inner = { start: start, end: end }; - this.outer = { start: start, end: end }; + this.range = { start: start, end: end }; } - /** - * Get or set (if called with an argument) the start time for queries. - * @param {number} [value] the start time to set - * @returns {number} the start time - */ - TimeConductor.prototype.queryStart = function (value) { - if (arguments.length > 0) { - this.outer.start = value; - } - return this.outer.start; - }; - - /** - * Get or set (if called with an argument) the end time for queries. - * @param {number} [value] the end time to set - * @returns {number} the end time - */ - TimeConductor.prototype.queryEnd = function (value) { - if (arguments.length > 0) { - this.outer.end = value; - } - return this.outer.end; - }; - - /** * Get or set (if called with an argument) the start time for displays. * @param {number} [value] the start time to set @@ -77,9 +51,9 @@ define( */ TimeConductor.prototype.displayStart = function (value) { if (arguments.length > 0) { - this.inner.start = value; + this.range.start = value; } - return this.inner.start; + return this.range.start; }; /** @@ -89,9 +63,9 @@ define( */ TimeConductor.prototype.displayEnd = function (value) { if (arguments.length > 0) { - this.inner.end = value; + this.range.end = value; } - return this.inner.end; + return this.range.end; }; return TimeConductor; diff --git a/platform/features/conductor/test/ConductorRepresenterSpec.js b/platform/features/conductor/test/ConductorRepresenterSpec.js index 2b1003f3c7..a0bb60c2de 100644 --- a/platform/features/conductor/test/ConductorRepresenterSpec.js +++ b/platform/features/conductor/test/ConductorRepresenterSpec.js @@ -77,7 +77,7 @@ define( mockElement = jasmine.createSpyObj('element', ELEMENT_METHODS); mockConductor = jasmine.createSpyObj( 'conductor', - [ 'queryStart', 'queryEnd', 'displayStart', 'displayEnd' ] + [ 'displayStart', 'displayEnd' ] ); mockCompiledTemplate = jasmine.createSpy('template'); mockNewScope = jasmine.createSpyObj('newScope', SCOPE_METHODS); @@ -127,15 +127,13 @@ define( }); it("exposes conductor state in scope", function () { - mockConductor.queryStart.andReturn(42); - mockConductor.queryEnd.andReturn(12321); mockConductor.displayStart.andReturn(1977); mockConductor.displayEnd.andReturn(1984); representer.represent(testViews[0], {}); expect(mockNewScope.conductor).toEqual({ inner: { start: 1977, end: 1984 }, - outer: { start: 42, end: 12321 } + outer: { start: 1977, end: 1984 } }); }); @@ -154,12 +152,6 @@ define( fireWatch(mockNewScope, 'conductor.inner.end', testState.inner.end); expect(mockConductor.displayEnd).toHaveBeenCalledWith(1984); - - fireWatch(mockNewScope, 'conductor.outer.start', testState.outer.start); - expect(mockConductor.queryStart).toHaveBeenCalledWith(-1977); - - fireWatch(mockNewScope, 'conductor.outer.end', testState.outer.end); - expect(mockConductor.queryEnd).toHaveBeenCalledWith(12321); }); }); diff --git a/platform/features/conductor/test/ConductorServiceSpec.js b/platform/features/conductor/test/ConductorServiceSpec.js index 5146ca5f42..640212540c 100644 --- a/platform/features/conductor/test/ConductorServiceSpec.js +++ b/platform/features/conductor/test/ConductorServiceSpec.js @@ -43,9 +43,9 @@ define( it("initializes a time conductor around the current time", function () { var conductor = conductorService.getConductor(); - expect(conductor.queryStart() <= TEST_NOW).toBeTruthy(); - expect(conductor.queryEnd() >= TEST_NOW).toBeTruthy(); - expect(conductor.queryEnd() > conductor.queryStart()) + expect(conductor.displayStart() <= TEST_NOW).toBeTruthy(); + expect(conductor.displayEnd() >= TEST_NOW).toBeTruthy(); + expect(conductor.displayEnd() > conductor.displayStart()) .toBeTruthy(); }); diff --git a/platform/features/conductor/test/TimeConductorSpec.js b/platform/features/conductor/test/TimeConductorSpec.js index 558322329e..1e3859bf60 100644 --- a/platform/features/conductor/test/TimeConductorSpec.js +++ b/platform/features/conductor/test/TimeConductorSpec.js @@ -41,19 +41,13 @@ define( }); it("provides accessors for query/display start/end times", function () { - expect(conductor.queryStart()).toEqual(testStart); - expect(conductor.queryEnd()).toEqual(testEnd); expect(conductor.displayStart()).toEqual(testStart); expect(conductor.displayEnd()).toEqual(testEnd); }); it("provides setters for query/display start/end times", function () { - expect(conductor.queryStart(1)).toEqual(1); - expect(conductor.queryEnd(2)).toEqual(2); expect(conductor.displayStart(3)).toEqual(3); expect(conductor.displayEnd(4)).toEqual(4); - expect(conductor.queryStart()).toEqual(1); - expect(conductor.queryEnd()).toEqual(2); expect(conductor.displayStart()).toEqual(3); expect(conductor.displayEnd()).toEqual(4); }); From 8cba3218866abe9e385f48260fea6952dd1c4b01 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 16:11:12 -0700 Subject: [PATCH 24/28] [Plot] Move throttling out of plot Move throttling associated with display bounds changes out of Plot. --- platform/features/plot/src/PlotController.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/platform/features/plot/src/PlotController.js b/platform/features/plot/src/PlotController.js index 5555d93659..19aee9ca11 100644 --- a/platform/features/plot/src/PlotController.js +++ b/platform/features/plot/src/PlotController.js @@ -66,7 +66,6 @@ define( cachedObjects = [], updater, lastBounds, - throttledRequery, handle; // Populate the scope with axis information (specifically, options @@ -188,15 +187,10 @@ define( function changeDisplayBounds(event, bounds) { self.pending = true; releaseSubscription(); - throttledRequery(); + subscribe($scope.domainObject); setBasePanZoom(bounds); } - // Reestablish/reissue request for telemetry - throttledRequery = throttle(function () { - subscribe($scope.domainObject); - }, 250); - this.modeOptions = new PlotModeOptions([], subPlotFactory); this.updateValues = updateValues; From cd0c0f77cc7bc38784afdc31c9e05e173d3f660d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 16:20:31 -0700 Subject: [PATCH 25/28] [Time Conductor] Throttle display bounds broadcasting --- platform/features/conductor/bundle.json | 7 +++- .../conductor/src/ConductorRepresenter.js | 34 ++++++++++++++----- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/platform/features/conductor/bundle.json b/platform/features/conductor/bundle.json index b230f9d370..2e4a7c652f 100644 --- a/platform/features/conductor/bundle.json +++ b/platform/features/conductor/bundle.json @@ -3,7 +3,12 @@ "representers": [ { "implementation": "ConductorRepresenter.js", - "depends": [ "conductorService", "$compile", "views[]" ] + "depends": [ + "throttle", + "conductorService", + "$compile", + "views[]" + ] } ], "components": [ diff --git a/platform/features/conductor/src/ConductorRepresenter.js b/platform/features/conductor/src/ConductorRepresenter.js index 212c2e6a71..cf26e8079f 100644 --- a/platform/features/conductor/src/ConductorRepresenter.js +++ b/platform/features/conductor/src/ConductorRepresenter.js @@ -36,6 +36,7 @@ define( "", '
' ].join(''), + THROTTLE_MS = 200, GLOBAL_SHOWING = false; /** @@ -45,6 +46,8 @@ define( * @implements {Representer} * @constructor * @memberof platform/features/conductor + * @param {Function} throttle a function used to reduce the frequency + * of function invocations * @param {platform/features/conductor.ConductorService} conductorService * service which provides the active time conductor * @param $compile Angular's $compile @@ -52,7 +55,15 @@ define( * @param {Scope} the scope of the representation * @param element the jqLite-wrapped representation element */ - function ConductorRepresenter(conductorService, $compile, views, scope, element) { + function ConductorRepresenter( + throttle, + conductorService, + $compile, + views, + scope, + element + ) { + this.throttle = throttle; this.scope = scope; this.conductorService = conductorService; this.element = element; @@ -61,7 +72,12 @@ define( } // Update the time conductor from the scope - function wireScope(conductor, conductorScope, repScope) { + ConductorRepresenter.prototype.wireScope = function () { + var conductor = this.conductorService.getConductor(), + conductorScope = this.conductorScope(), + repScope = this.scope, + broadcastBounds; + // Combine start/end times into a single object function bounds(start, end) { return { @@ -73,9 +89,13 @@ define( function updateConductorInner() { conductor.displayStart(conductorScope.conductor.inner.start); conductor.displayEnd(conductorScope.conductor.inner.end); - repScope.$broadcast('telemetry:display:bounds', bounds()); + broadcastBounds(); } + broadcastBounds = this.throttle(function () { + repScope.$broadcast('telemetry:display:bounds', bounds()); + }, THROTTLE_MS); + conductorScope.conductor = { outer: bounds(), inner: bounds() }; conductorScope @@ -84,7 +104,7 @@ define( .$watch('conductor.inner.end', updateConductorInner); repScope.$on('telemetry:view', updateConductorInner); - } + }; ConductorRepresenter.prototype.conductorScope = function (s) { return (this.cScope = arguments.length > 0 ? s : this.cScope); @@ -101,11 +121,7 @@ define( // Create a new scope for the conductor this.conductorScope(this.scope.$new()); - wireScope( - this.conductorService.getConductor(), - this.conductorScope(), - this.scope - ); + this.wireScope(); this.conductorElement = this.$compile(TEMPLATE)(this.conductorScope()); this.element.after(this.conductorElement[0]); From 431c74ca49a486d7c0dd7a3255df5a55f6a992a7 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 16:28:37 -0700 Subject: [PATCH 26/28] [Time Conductor] Only update bounds when stable --- .../conductor/src/ConductorRepresenter.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/platform/features/conductor/src/ConductorRepresenter.js b/platform/features/conductor/src/ConductorRepresenter.js index cf26e8079f..ae6fdf2665 100644 --- a/platform/features/conductor/src/ConductorRepresenter.js +++ b/platform/features/conductor/src/ConductorRepresenter.js @@ -76,6 +76,7 @@ define( var conductor = this.conductorService.getConductor(), conductorScope = this.conductorScope(), repScope = this.scope, + lastObservedBounds, broadcastBounds; // Combine start/end times into a single object @@ -86,14 +87,29 @@ define( }; } + function boundsAreStable(newlyObservedBounds) { + return !lastObservedBounds || + (lastObservedBounds.start === newlyObservedBounds.start && + lastObservedBounds.end === newlyObservedBounds.end); + } + function updateConductorInner() { conductor.displayStart(conductorScope.conductor.inner.start); conductor.displayEnd(conductorScope.conductor.inner.end); + lastObservedBounds = lastObservedBounds || bounds(); broadcastBounds(); } broadcastBounds = this.throttle(function () { - repScope.$broadcast('telemetry:display:bounds', bounds()); + var newlyObservedBounds = bounds(); + + if (boundsAreStable(newlyObservedBounds)) { + repScope.$broadcast('telemetry:display:bounds', bounds()); + lastObservedBounds = undefined; + } else { + lastObservedBounds = newlyObservedBounds; + broadcastBounds(); + } }, THROTTLE_MS); conductorScope.conductor = { outer: bounds(), inner: bounds() }; From 669b434c362a0439b982dbec969a0b26fbde9f13 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 16:38:32 -0700 Subject: [PATCH 27/28] [Time Conductor] Test broadcast throttling --- .../test/ConductorRepresenterSpec.js | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/platform/features/conductor/test/ConductorRepresenterSpec.js b/platform/features/conductor/test/ConductorRepresenterSpec.js index a0bb60c2de..59fae1b4ee 100644 --- a/platform/features/conductor/test/ConductorRepresenterSpec.js +++ b/platform/features/conductor/test/ConductorRepresenterSpec.js @@ -47,7 +47,8 @@ define( ]; describe("ConductorRepresenter", function () { - var mockConductorService, + var mockThrottle, + mockConductorService, mockCompile, testViews, mockScope, @@ -67,6 +68,7 @@ define( } beforeEach(function () { + mockThrottle = jasmine.createSpy('throttle'); mockConductorService = jasmine.createSpyObj( 'conductorService', ['getConductor'] @@ -88,8 +90,12 @@ define( mockCompile.andReturn(mockCompiledTemplate); mockCompiledTemplate.andReturn(mockNewElement); mockScope.$new.andReturn(mockNewScope); + mockThrottle.andCallFake(function (fn) { + return fn; + }); representer = new ConductorRepresenter( + mockThrottle, mockConductorService, mockCompile, testViews, @@ -154,6 +160,56 @@ define( expect(mockConductor.displayEnd).toHaveBeenCalledWith(1984); }); + describe("when bounds are changing", function () { + var mockThrottledFn = jasmine.createSpy('throttledFn'), + testBounds; + + function fireThrottledFn() { + mockThrottle.mostRecentCall.args[0](); + } + + beforeEach(function () { + mockThrottle.andReturn(mockThrottledFn); + representer.represent(testViews[0], {}); + testBounds = { start: 0, end: 1000 }; + mockNewScope.conductor.inner = testBounds; + mockConductor.displayStart.andCallFake(function () { + return testBounds.start; + }); + mockConductor.displayEnd.andCallFake(function () { + return testBounds.end; + }); + }); + + it("does not broadcast while bounds are changing", function () { + expect(mockScope.$broadcast).not.toHaveBeenCalled(); + testBounds.start = 100; + fireWatch(mockNewScope, 'conductor.inner.start', testBounds.start); + testBounds.end = 500; + fireWatch(mockNewScope, 'conductor.inner.end', testBounds.end); + fireThrottledFn(); + testBounds.start = 200; + fireWatch(mockNewScope, 'conductor.inner.start', testBounds.start); + testBounds.end = 400; + fireWatch(mockNewScope, 'conductor.inner.end', testBounds.end); + fireThrottledFn(); + expect(mockScope.$broadcast).not.toHaveBeenCalled(); + }); + + it("does broadcast when bounds have stabilized", function () { + expect(mockScope.$broadcast).not.toHaveBeenCalled(); + testBounds.start = 100; + fireWatch(mockNewScope, 'conductor.inner.start', testBounds.start); + testBounds.end = 500; + fireWatch(mockNewScope, 'conductor.inner.end', testBounds.end); + fireThrottledFn(); + fireWatch(mockNewScope, 'conductor.inner.start', testBounds.start); + fireWatch(mockNewScope, 'conductor.inner.end', testBounds.end); + fireThrottledFn(); + expect(mockScope.$broadcast).toHaveBeenCalled(); + }); + }); + }); } ); From 146e948097ec2c81b6a32e6ab7d6e58bff2c234c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 2 Oct 2015 16:46:42 -0700 Subject: [PATCH 28/28] [Time Conductor] Remove telemetry series wrapper WTD-1515 --- .../src/ConductorTelemetryDecorator.js | 40 +-------- .../conductor/src/ConductorTelemetrySeries.js | 71 --------------- .../test/ConductorTelemetryDecoratorSpec.js | 21 ----- .../test/ConductorTelemetrySeriesSpec.js | 86 ------------------- platform/features/conductor/test/suite.json | 1 - 5 files changed, 3 insertions(+), 216 deletions(-) delete mode 100644 platform/features/conductor/src/ConductorTelemetrySeries.js delete mode 100644 platform/features/conductor/test/ConductorTelemetrySeriesSpec.js diff --git a/platform/features/conductor/src/ConductorTelemetryDecorator.js b/platform/features/conductor/src/ConductorTelemetryDecorator.js index 9c8126c33f..ed16db7d1b 100644 --- a/platform/features/conductor/src/ConductorTelemetryDecorator.js +++ b/platform/features/conductor/src/ConductorTelemetryDecorator.js @@ -22,8 +22,7 @@ /*global define*/ define( - ['./ConductorTelemetrySeries'], - function (ConductorTelemetrySeries) { + function () { 'use strict'; /** @@ -42,32 +41,6 @@ define( this.telemetryService = telemetryService; } - // Strip out any realtime data series that is outside of the conductor's - // bounds. - ConductorTelemetryDecorator.prototype.pruneNonDisplayable = function (packaged) { - var conductor = this.conductorService.getConductor(), - repackaged = {}; - - function filterSource(packagedBySource) { - var repackagedBySource = {}; - - Object.keys(packagedBySource).forEach(function (k) { - repackagedBySource[k] = new ConductorTelemetrySeries( - packagedBySource[k], - conductor - ); - }); - - return repackagedBySource; - } - - Object.keys(packaged).forEach(function (source) { - repackaged[source] = filterSource(packaged[source]); - }); - - return repackaged; - }; - ConductorTelemetryDecorator.prototype.amendRequests = function (requests) { var conductor = this.conductorService.getConductor(), start = conductor.displayStart(), @@ -86,21 +59,14 @@ define( ConductorTelemetryDecorator.prototype.requestTelemetry = function (requests) { var self = this; return this.telemetryService - .requestTelemetry(this.amendRequests(requests)) - .then(function (packaged) { - return self.pruneNonDisplayable(packaged); - }); + .requestTelemetry(this.amendRequests(requests)); }; ConductorTelemetryDecorator.prototype.subscribe = function (callback, requests) { var self = this; - function internalCallback(packagedSeries) { - return callback(self.pruneNonDisplayable(packagedSeries)); - } - return this.telemetryService - .subscribe(internalCallback, this.amendRequests(requests)); + .subscribe(callback, this.amendRequests(requests)); }; return ConductorTelemetryDecorator; diff --git a/platform/features/conductor/src/ConductorTelemetrySeries.js b/platform/features/conductor/src/ConductorTelemetrySeries.js deleted file mode 100644 index aa6ec0ec63..0000000000 --- a/platform/features/conductor/src/ConductorTelemetrySeries.js +++ /dev/null @@ -1,71 +0,0 @@ -/***************************************************************************** - * 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*/ - -define( - function () { - 'use strict'; - - - /** - * Bound a series of telemetry such that it only includes - * points from within the time conductor's displayable window. - * - * @param {TelemetrySeries} series the telemetry series - * @param {platform/features/conductor.TimeConductor} the - * time conductor instance which bounds this series - * @constructor - * @implements {TelemetrySeries} - */ - function ConductorTelemetrySeries(series, conductor) { - var max = series.getPointCount() - 1; - - function binSearch(min, max, value) { - var mid = Math.floor((min + max) / 2); - - return min > max ? min : - series.getDomainValue(mid) < value ? - binSearch(mid + 1, max, value) : - binSearch(min, mid - 1, value); - } - - this.startIndex = binSearch(0, max, conductor.displayStart()); - this.endIndex = binSearch(0, max, conductor.displayEnd()); - this.series = series; - } - - ConductorTelemetrySeries.prototype.getPointCount = function () { - return Math.max(0, this.endIndex - this.startIndex); - }; - - ConductorTelemetrySeries.prototype.getDomainValue = function (i, d) { - return this.series.getDomainValue(i + this.startIndex, d); - }; - - ConductorTelemetrySeries.prototype.getRangeValue = function (i, r) { - return this.series.getRangeValue(i + this.startIndex, r); - }; - - return ConductorTelemetrySeries; - } -); diff --git a/platform/features/conductor/test/ConductorTelemetryDecoratorSpec.js b/platform/features/conductor/test/ConductorTelemetryDecoratorSpec.js index 84812b541a..9a5efc2448 100644 --- a/platform/features/conductor/test/ConductorTelemetryDecoratorSpec.js +++ b/platform/features/conductor/test/ConductorTelemetryDecoratorSpec.js @@ -110,27 +110,6 @@ define( }]); }); - it("prunes historical values to the displayable range", function () { - var packagedTelemetry; - decorator.requestTelemetry([{ source: "abc", key: "xyz" }]); - packagedTelemetry = mockPromise.then.mostRecentCall.args[0]({ - "abc": { "xyz": mockSeries } - }); - expect(seriesIsInWindow(packagedTelemetry.abc.xyz)) - .toBeTruthy(); - }); - - it("prunes subscribed values to the displayable range", function () { - var mockCallback = jasmine.createSpy('callback'), - packagedTelemetry; - decorator.subscribe(mockCallback, [{ source: "abc", key: "xyz" }]); - mockTelemetryService.subscribe.mostRecentCall.args[0]({ - "abc": { "xyz": mockSeries } - }); - packagedTelemetry = mockCallback.mostRecentCall.args[0]; - expect(seriesIsInWindow(packagedTelemetry.abc.xyz)) - .toBeTruthy(); - }); }); } diff --git a/platform/features/conductor/test/ConductorTelemetrySeriesSpec.js b/platform/features/conductor/test/ConductorTelemetrySeriesSpec.js deleted file mode 100644 index ea884f74f3..0000000000 --- a/platform/features/conductor/test/ConductorTelemetrySeriesSpec.js +++ /dev/null @@ -1,86 +0,0 @@ -/***************************************************************************** - * 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,describe,it,expect,beforeEach,waitsFor,jasmine*/ - -define( - ["../src/ConductorTelemetrySeries"], - function (ConductorTelemetrySeries) { - "use strict"; - - describe("ConductorTelemetrySeries", function () { - var mockSeries, - mockConductor, - testArray, - series; - - beforeEach(function () { - testArray = [ -10, 0, 42, 1977, 12321 ]; - - mockSeries = jasmine.createSpyObj( - 'series', - [ 'getPointCount', 'getDomainValue', 'getRangeValue' ] - ); - mockConductor = jasmine.createSpyObj( - 'conductor', - [ 'queryStart', 'queryEnd', 'displayStart', 'displayEnd' ] - ); - - mockSeries.getPointCount.andCallFake(function () { - return testArray.length; - }); - mockSeries.getDomainValue.andCallFake(function (i) { - return testArray[i]; - }); - mockSeries.getRangeValue.andCallFake(function (i) { - return testArray[i] * 2; - }); - - mockConductor.displayStart.andReturn(0); - mockConductor.displayEnd.andReturn(2000); - - series = new ConductorTelemetrySeries( - mockSeries, - mockConductor - ); - }); - - it("reduces the apparent size of a series", function () { - expect(series.getPointCount()).toEqual(3); - }); - - it("maps domain value indexes to the displayable range", function () { - [0, 1, 2].forEach(function (i) { - expect(series.getDomainValue(i)) - .toEqual(mockSeries.getDomainValue(i + 1)); - }); - }); - - it("maps range value indexes to the displayable range", function () { - [0, 1, 2].forEach(function (i) { - expect(series.getRangeValue(i)) - .toEqual(mockSeries.getRangeValue(i + 1)); - }); - }); - - }); - } -); diff --git a/platform/features/conductor/test/suite.json b/platform/features/conductor/test/suite.json index 0c469617de..9343b8e422 100644 --- a/platform/features/conductor/test/suite.json +++ b/platform/features/conductor/test/suite.json @@ -2,6 +2,5 @@ "ConductorRepresenter", "ConductorService", "ConductorTelemetryDecorator", - "ConductorTelemetrySeries", "TimeConductor" ]