From 53aabd57437981a7c2d723bd6c2c5ac4cc515926 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 10:33:27 -0800 Subject: [PATCH 01/21] [Fixed Position] Add ordering control Add order control for elements in a fixed position view, WTD-881. --- platform/features/layout/bundle.json | 32 +++++++++++++++++ .../layout/src/elements/ElementProxy.js | 34 +++++++++++++++++++ .../layout/test/elements/ElementProxySpec.js | 11 ++++++ 3 files changed, 77 insertions(+) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index d7fb512219..6a9e2fdc49 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -55,6 +55,38 @@ } ] }, + { + "items": [ + { + "method": "order", + "glyph": "o", + "control": "menu-button", + "inclusive": true, + "options": [ + { + "name": "Move to Top", + "glyph": "^", + "key": "top" + }, + { + "name": "Move Up", + "glyph": "^", + "key": "up" + }, + { + "name": "Move Down", + "glyph": "v", + "key": "down" + }, + { + "name": "Move to Bottom", + "glyph": "v", + "key": "bottom" + } + ] + } + ] + }, { "items": [ { diff --git a/platform/features/layout/src/elements/ElementProxy.js b/platform/features/layout/src/elements/ElementProxy.js index 277353e315..438725f384 100644 --- a/platform/features/layout/src/elements/ElementProxy.js +++ b/platform/features/layout/src/elements/ElementProxy.js @@ -5,11 +5,23 @@ define( function (AccessorMutator) { "use strict"; + // Index deltas for changes in order + var ORDERS = { + top: Number.POSITIVE_INFINITY, + up: 1, + down: -1, + bottom: Number.NEGATIVE_INFINITY + }; + /** * Abstract superclass for other classes which provide useful * interfaces upon an elements in a fixed position view. * This handles the generic operations (e.g. remove) so that * subclasses only need to implement element-specific behaviors. + * + * Note that arguments here are meant to match those expected + * by `Array.prototype.map` + * * @constructor * @param element the fixed position element, as stored in its * configuration @@ -56,6 +68,28 @@ define( * @returns {number} the height */ height: new AccessorMutator(element, 'height'), + /** + * Change the display order of this element. + * @param {string} o where to move this element; + * one of "top", "up", "down", or "bottom" + */ + order: function (o) { + var delta = ORDERS[o] || 0, + desired = Math.max( + Math.min(index + delta, elements.length - 1), + 0 + ); + // Move to the desired index, if this is a change + if ((desired !== index) && (elements[index] === element)) { + // Splice out the current element + elements.splice(index, 1); + // Splice it back in at the correct index + elements.splice(desired, 0, element); + // Track change in index (proxy should be recreated + // anyway, but be consistent) + index = desired; + } + }, /** * Remove this element from the fixed position view. */ diff --git a/platform/features/layout/test/elements/ElementProxySpec.js b/platform/features/layout/test/elements/ElementProxySpec.js index e2a9c2880e..32e819ace8 100644 --- a/platform/features/layout/test/elements/ElementProxySpec.js +++ b/platform/features/layout/test/elements/ElementProxySpec.js @@ -36,6 +36,17 @@ define( proxy.remove(); expect(testElements).toEqual([{}, {}, {}]); }); + + it("allows order to be changed", function () { + proxy.order("down"); + expect(testElements).toEqual([{}, testElement, {}, {}]); + proxy.order("up"); + expect(testElements).toEqual([{}, {}, testElement, {}]); + proxy.order("bottom"); + expect(testElements).toEqual([testElement, {}, {}, {}]); + proxy.order("top"); + expect(testElements).toEqual([{}, {}, {}, testElement]); + }); }); } ); From 182d02599dd1d8e3901365a10045dc683df3103d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 10:48:44 -0800 Subject: [PATCH 02/21] [Fixed Position] Add fill properties Add 'fill' properties, to allow specifying background color in Fixed Position view. WTD-881. --- .../res/templates/elements/telemetry.html | 3 +- .../features/layout/src/elements/BoxProxy.js | 40 +++++++++++++++++++ .../layout/src/elements/TelemetryProxy.js | 16 ++++++-- .../features/layout/src/elements/TextProxy.js | 28 +++++++++++++ .../layout/test/elements/BoxProxySpec.js | 36 +++++++++++++++++ .../layout/test/elements/TextProxySpec.js | 36 +++++++++++++++++ platform/features/layout/test/suite.json | 4 +- 7 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 platform/features/layout/src/elements/BoxProxy.js create mode 100644 platform/features/layout/src/elements/TextProxy.js create mode 100644 platform/features/layout/test/elements/BoxProxySpec.js create mode 100644 platform/features/layout/test/elements/TextProxySpec.js diff --git a/platform/features/layout/res/templates/elements/telemetry.html b/platform/features/layout/res/templates/elements/telemetry.html index cb12e916b7..f5b51a0311 100644 --- a/platform/features/layout/res/templates/elements/telemetry.html +++ b/platform/features/layout/res/templates/elements/telemetry.html @@ -1,4 +1,5 @@ -
+
{{ngModel.name}}
diff --git a/platform/features/layout/src/elements/BoxProxy.js b/platform/features/layout/src/elements/BoxProxy.js new file mode 100644 index 0000000000..5cfbb6ef12 --- /dev/null +++ b/platform/features/layout/src/elements/BoxProxy.js @@ -0,0 +1,40 @@ +/*global define*/ + +define( + ['./ElementProxy', './AccessorMutator'], + function (ElementProxy, AccessorMutator) { + 'use strict'; + + /** + * Selection proxy for Box elements in a fixed position view. + * Also serves as a superclass for Text elements, since those + * elements have a superset of Box properties. + * + * Note that arguments here are meant to match those expected + * by `Array.prototype.map` + * + * @constructor + * @param element the fixed position element, as stored in its + * configuration + * @param index the element's index within its array + * @param {Array} elements the full array of elements + */ + function BoxProxy(element, index, elements) { + var proxy = new ElementProxy(element, index, elements); + + /** + * Get/set this element's fill color. (Omitting the + * argument makes this act as a getter.) + * @method + * @memberof BoxProxy + * @param {string} fill the new fill color + * @returns {string} the fill color + */ + proxy.fill = new AccessorMutator(element, 'fill'); + + return proxy; + } + + return BoxProxy; + } +); \ No newline at end of file diff --git a/platform/features/layout/src/elements/TelemetryProxy.js b/platform/features/layout/src/elements/TelemetryProxy.js index 610edb895c..b587465df4 100644 --- a/platform/features/layout/src/elements/TelemetryProxy.js +++ b/platform/features/layout/src/elements/TelemetryProxy.js @@ -1,16 +1,26 @@ /*global define*/ define( - ['./ElementProxy'], - function (ElementProxy) { + ['./TextProxy', './AccessorMutator'], + function (TextProxy, AccessorMutator) { 'use strict'; /** + * Selection proxy for telemetry elements in a fixed position view. * + * Note that arguments here are meant to match those expected + * by `Array.prototype.map` + * + * @constructor + * @param element the fixed position element, as stored in its + * configuration + * @param index the element's index within its array + * @param {Array} elements the full array of elements */ function TelemetryProxy(element, index, elements) { - var proxy = new ElementProxy(element, index, elements); + var proxy = new TextProxy(element, index, elements); + // Expose the domain object identifier proxy.id = element.id; return proxy; diff --git a/platform/features/layout/src/elements/TextProxy.js b/platform/features/layout/src/elements/TextProxy.js new file mode 100644 index 0000000000..53ba553672 --- /dev/null +++ b/platform/features/layout/src/elements/TextProxy.js @@ -0,0 +1,28 @@ +/*global define*/ + +define( + ['./BoxProxy', './AccessorMutator'], + function (BoxProxy, AccessorMutator) { + 'use strict'; + + /** + * Selection proxy for Text elements in a fixed position view. + * + * Note that arguments here are meant to match those expected + * by `Array.prototype.map` + * + * @constructor + * @param element the fixed position element, as stored in its + * configuration + * @param index the element's index within its array + * @param {Array} elements the full array of elements + */ + function TextProxy(element, index, elements) { + var proxy = new BoxProxy(element, index, elements); + + return proxy; + } + + return TextProxy; + } +); \ No newline at end of file diff --git a/platform/features/layout/test/elements/BoxProxySpec.js b/platform/features/layout/test/elements/BoxProxySpec.js new file mode 100644 index 0000000000..cf37b6f5db --- /dev/null +++ b/platform/features/layout/test/elements/BoxProxySpec.js @@ -0,0 +1,36 @@ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ['../../src/elements/BoxProxy'], + function (BoxProxy) { + "use strict"; + + describe("A fixed position box proxy", function () { + var testElement, + testElements, + proxy; + + beforeEach(function () { + testElement = { + x: 1, + y: 2, + width: 42, + height: 24, + fill: "transparent" + }; + testElements = [ {}, {}, testElement, {} ]; + proxy = new BoxProxy( + testElement, + testElements.indexOf(testElement), + testElements + ); + }); + + it("provides getter/setter for fill color", function () { + expect(proxy.fill()).toEqual('transparent'); + expect(proxy.fill('#FFF')).toEqual('#FFF'); + expect(proxy.fill()).toEqual('#FFF'); + }); + }); + } +); diff --git a/platform/features/layout/test/elements/TextProxySpec.js b/platform/features/layout/test/elements/TextProxySpec.js new file mode 100644 index 0000000000..0910397702 --- /dev/null +++ b/platform/features/layout/test/elements/TextProxySpec.js @@ -0,0 +1,36 @@ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ['../../src/elements/TextProxy'], + function (TextProxy) { + "use strict"; + + describe("A fixed position text proxy", function () { + var testElement, + testElements, + proxy; + + beforeEach(function () { + testElement = { + x: 1, + y: 2, + width: 42, + height: 24, + fill: "transparent" + }; + testElements = [ {}, {}, testElement, {} ]; + proxy = new TextProxy( + testElement, + testElements.indexOf(testElement), + testElements + ); + }); + + it("provides getter/setter for fill color", function () { + expect(proxy.fill()).toEqual('transparent'); + expect(proxy.fill('#FFF')).toEqual('#FFF'); + expect(proxy.fill()).toEqual('#FFF'); + }); + }); + } +); diff --git a/platform/features/layout/test/suite.json b/platform/features/layout/test/suite.json index c895900595..8f0eec06c3 100644 --- a/platform/features/layout/test/suite.json +++ b/platform/features/layout/test/suite.json @@ -5,9 +5,11 @@ "LayoutDrag", "LayoutSelection", "elements/AccessorMutator", + "elements/BoxProxy", "elements/ElementFactory", "elements/ElementProxies", "elements/ElementProxy", "elements/LineProxy", - "elements/TelemetryProxy" + "elements/TelemetryProxy", + "elements/TextProxy" ] \ No newline at end of file From c183f08886ab677ee12e76a048e01d71977eb814 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 14:51:17 -0800 Subject: [PATCH 03/21] [Edit] Fix state-watching Repair/simplify state-watching in representer for toolbar in edit mode; avoids issue where destroy calls detach watches for state change in toolbar prematurely. WTD-881. --- platform/commonUI/edit/src/EditRepresenter.js | 1 - .../representers/EditToolbarRepresenter.js | 34 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/platform/commonUI/edit/src/EditRepresenter.js b/platform/commonUI/edit/src/EditRepresenter.js index bf3dfc721b..5e3f3a8606 100644 --- a/platform/commonUI/edit/src/EditRepresenter.js +++ b/platform/commonUI/edit/src/EditRepresenter.js @@ -76,7 +76,6 @@ define( key = (representation || {}).key; // Track the represented object domainObject = representedObject; - // Ensure existing watches are released destroy(); } diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index 8e4958d5df..63a4c9d3cd 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -42,6 +42,11 @@ define( toolbarObject.state = toolbar.getState(); } + // Get state (to watch it) + function getState() { + return toolbarObject.state; + } + // Update selection models to match changed toolbar state function updateState(state) { // Update underlying state based on toolbar changes @@ -52,8 +57,18 @@ define( commit("Changes from toolbar."); } + function initialize() { + // If we have been asked to expose toolbar state... + if (attrs.toolbar) { + // Expose toolbar state under that name + scope.$parent[attrs.toolbar] = toolbarObject; + } + } + // Represent a domain object using this definition function represent(representation) { + // + initialize(); // Clear any existing selection scope.selection = []; // Get the newest toolbar definition from the view @@ -62,24 +77,19 @@ define( updateSelection([]); } - // Destroy; stop watching the parent for changes in - // toolbar state. + // Destroy; remove toolbar object from parent scope function destroy() { - if (unwatch) { - unwatch(); - unwatch = undefined; + // Clear exposed toolbar state (if any) + if (attrs.toolbar) { + delete scope.$parent[attrs.toolbar]; } } - // If we have been asked to expose toolbar state... + // If this representation exposes a toolbar, set up watches + // to synchronize with it. if (attrs.toolbar) { - // Expose toolbar state under that name - scope.$parent[attrs.toolbar] = toolbarObject; // Detect and handle changes to state from the toolbar - unwatch = scope.$parent.$watchCollection( - attrs.toolbar + ".state", - updateState - ); + scope.$watchCollection(getState, updateState); // Watch for changes in the current selection state scope.$watchCollection("selection", updateSelection); } From 46c8a456b7dd3629494e063be878a05f34bba715 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 14:52:19 -0800 Subject: [PATCH 04/21] [Fixed Position] Begin adding color chooser Begin adding choice of fill colors for box/text/telemetry elements in a fixed position view, WTD-881. --- platform/features/layout/bundle.json | 6 ++++++ platform/features/layout/src/elements/ElementProxies.js | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index 6a9e2fdc49..a42bef49b8 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -84,6 +84,12 @@ "key": "bottom" } ] + }, + { + "property": "fill", + "glyph": "X", + "control": "textfield", + "inclusive": true } ] }, diff --git a/platform/features/layout/src/elements/ElementProxies.js b/platform/features/layout/src/elements/ElementProxies.js index 5f71e89916..125d0c7f10 100644 --- a/platform/features/layout/src/elements/ElementProxies.js +++ b/platform/features/layout/src/elements/ElementProxies.js @@ -1,16 +1,16 @@ /*global define*/ define( - ['./TelemetryProxy', './ElementProxy', './LineProxy'], - function (TelemetryProxy, ElementProxy, LineProxy) { + ['./TelemetryProxy', './ElementProxy', './LineProxy', './BoxProxy', './TextProxy'], + function (TelemetryProxy, ElementProxy, LineProxy, BoxProxy, TextProxy) { "use strict"; return { "fixed.telemetry": TelemetryProxy, "fixed.line": LineProxy, - "fixed.box": ElementProxy, + "fixed.box": BoxProxy, "fixed.image": ElementProxy, - "fixed.text": ElementProxy + "fixed.text": TextProxy }; } ); \ No newline at end of file From f89423318242c2ad683ccd0f0f24d7a3bbe3327c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 15:13:46 -0800 Subject: [PATCH 05/21] [Edit] Default to inclusive mode Default to inclusive mode in Edit mode toolbar; simplifies adding additional toolbar elements for Fixed Position view, WTD-881. --- platform/commonUI/edit/README.md | 8 ++++---- platform/commonUI/edit/src/representers/EditToolbar.js | 2 +- .../edit/src/representers/EditToolbarRepresenter.js | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/edit/README.md b/platform/commonUI/edit/README.md index a8c552fb3e..7ddf90b699 100644 --- a/platform/commonUI/edit/README.md +++ b/platform/commonUI/edit/README.md @@ -17,10 +17,10 @@ view's scope.) These additional properties are: an argument to set.) * `method`: Name of a method to invoke upon a selected object when a control is activated, e.g. on a button click. -* `inclusive`: Optional; true if this control should be considered - applicable whenever at least one element in the selection has - the associated property. Otherwise, all members of the current - selection must have this property for the control to be shown. +* `exclusive`: Optional; true if this control should be considered + applicable only when all elements in the selection has + the associated property. Otherwise, only at least one member of the + current selection must have this property for the control to be shown. Controls in the toolbar are shown based on applicability to the current selection. Applicability for a given member of the selection diff --git a/platform/commonUI/edit/src/representers/EditToolbar.js b/platform/commonUI/edit/src/representers/EditToolbar.js index 60d6990bb6..4b2a925019 100644 --- a/platform/commonUI/edit/src/representers/EditToolbar.js +++ b/platform/commonUI/edit/src/representers/EditToolbar.js @@ -108,7 +108,7 @@ define( function isApplicable(item) { var property = (item || {}).property, method = (item || {}).method, - exclusive = !(item || {}).inclusive; + exclusive = (item || {}).exclusive; // Check if a selected item defines this property function hasProperty(selected) { diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index 63a4c9d3cd..0c5cd772d4 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -57,6 +57,7 @@ define( commit("Changes from toolbar."); } + // Initialize toolbar (expose object to parent scope) function initialize() { // If we have been asked to expose toolbar state... if (attrs.toolbar) { @@ -67,7 +68,7 @@ define( // Represent a domain object using this definition function represent(representation) { - // + // Expose the toolbar object to the parent scope initialize(); // Clear any existing selection scope.selection = []; From 6c7dfb878b8c88d9600bae30da0be40b8a274233 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 15:57:45 -0800 Subject: [PATCH 06/21] [Edit] Avoid rebuilding toolbar on edit Avoid rebuilding the toolbar in Edit mode whenever any change occurs to avoid losing the binding to the current selection state; needed for color picker to work properly for WTD-881. --- .../edit/src/representers/EditToolbar.js | 61 ++++++++++++++++--- .../representers/EditToolbarRepresenter.js | 37 +++++------ platform/forms/res/templates/toolbar.html | 2 + 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/platform/commonUI/edit/src/representers/EditToolbar.js b/platform/commonUI/edit/src/representers/EditToolbar.js index 4b2a925019..0fd916d561 100644 --- a/platform/commonUI/edit/src/representers/EditToolbar.js +++ b/platform/commonUI/edit/src/representers/EditToolbar.js @@ -20,9 +20,10 @@ define( * @param {Function} commit callback to invoke after changes * @constructor */ - function EditToolbar(structure, selection, commit) { + function EditToolbar(structure, commit) { var toolbarStructure = Object.create(structure || {}), toolbarState, + selection, properties = []; // Generate a new key for an item's property @@ -155,35 +156,79 @@ define( return section && section.items && section.items.length > 0; } - // Prepare a toolbar section based on current selection + // Prepare a toolbar section function convertSection(section) { var converted = Object.create(section || {}); converted.items = ((section || {}).items || []) - .map(convertItem) - .filter(isApplicable); + .map(convertItem); return converted; } + // Show/hide controls in this section per applicability + function refreshSectionApplicability(section) { + var count = 0; + // Show/hide each item + (section.items || []).forEach(function (item) { + item.hidden = !isApplicable(item); + count += item.hidden ? 0 : 1; + }); + // Hide this section if there are no applicable items + section.hidden = !count; + } + + // Show/hide controls if they are applicable + function refreshApplicability() { + toolbarStructure.sections.forEach(refreshSectionApplicability); + } + + // Refresh toolbar state to match selection + function refreshState() { + toolbarState = properties.map(initializeState); + } + toolbarStructure.sections = ((structure || {}).sections || []) .map(convertSection) .filter(nonEmpty); - toolbarState = properties.map(initializeState); + toolbarState = []; return { /** - * + * Set the current selection. Visisbility of sections + * and items in the toolbar will be updated to match this. + * @param {Array} s the new selection + */ + setSelection: function (s) { + selection = s; + refreshApplicability(); + refreshState(); + }, + /** + * Get the structure of the toolbar, as appropriate to + * pass to `mct-toolbar`. + * @returns the toolbar structure */ getStructure: function () { return toolbarStructure; }, + /** + * Get the current state of the toolbar, as appropriate + * to two-way bind to the state handled by `mct-toolbar`. + * @returns {Array} state of the toolbar + */ getState: function () { return toolbarState; }, - updateState: function (key, value) { - updateProperties(properties[key], value); + /** + * Update state within the current selection. + * @param {number} index the index of the corresponding + * element in the state array + * @param value the new value to convey to the selection + */ + updateState: function (index, value) { + updateProperties(properties[index], value); } }; } diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index 0c5cd772d4..5f2b740b72 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -15,9 +15,7 @@ define( * @constructor */ function EditToolbarRepresenter(scope, element, attrs) { - var definition, - unwatch, - toolbar, + var toolbar, toolbarObject = {}; // Mark changes as ready to persist @@ -29,17 +27,20 @@ define( // Handle changes to the current selection function updateSelection(selection) { - // Make sure selection is array-like - selection = Array.isArray(selection) ? - selection : - (selection ? [selection] : []); + // Only update if there is a toolbar to update + if (toolbar) { + // Make sure selection is array-like + selection = Array.isArray(selection) ? + selection : + (selection ? [selection] : []); - // Instantiate a new toolbar... - toolbar = new EditToolbar(definition, selection, commit); + // Update the toolbar's selection + toolbar.setSelection(selection); - // ...and expose its structure/state - toolbarObject.structure = toolbar.getStructure(); - toolbarObject.state = toolbar.getState(); + // ...and expose its structure/state + toolbarObject.structure = toolbar.getStructure(); + toolbarObject.state = toolbar.getState(); + } } // Get state (to watch it) @@ -50,7 +51,7 @@ define( // Update selection models to match changed toolbar state function updateState(state) { // Update underlying state based on toolbar changes - state.forEach(function (value, index) { + (state || []).forEach(function (value, index) { toolbar.updateState(index, value); }); // Commit the changes. @@ -58,9 +59,11 @@ define( } // Initialize toolbar (expose object to parent scope) - function initialize() { + function initialize(definition) { // If we have been asked to expose toolbar state... if (attrs.toolbar) { + // Initialize toolbar object + toolbar = new EditToolbar(definition, commit); // Expose toolbar state under that name scope.$parent[attrs.toolbar] = toolbarObject; } @@ -68,12 +71,12 @@ define( // Represent a domain object using this definition function represent(representation) { + // Get the newest toolbar definition from the view + var definition = (representation || {}).toolbar || {}; // Expose the toolbar object to the parent scope - initialize(); + initialize(definition); // Clear any existing selection scope.selection = []; - // Get the newest toolbar definition from the view - definition = (representation || {}).toolbar || {}; // Initialize toolbar to an empty selection updateSelection([]); } diff --git a/platform/forms/res/templates/toolbar.html b/platform/forms/res/templates/toolbar.html index 17602268d8..aa61bc3911 100644 --- a/platform/forms/res/templates/toolbar.html +++ b/platform/forms/res/templates/toolbar.html @@ -3,10 +3,12 @@
From 00f85447a2e68aa4461dc30876ff57e4b9eaeb2f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 16:47:52 -0800 Subject: [PATCH 07/21] [Fixed Position] Add color chooser Add color picker for fill color, WTD-881 --- platform/features/layout/bundle.json | 2 +- platform/forms/bundle.json | 4 + .../forms/res/templates/controls/color.html | 39 ++++++++- .../forms/src/controllers/ColorController.js | 81 +++++++++++++++++++ 4 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 platform/forms/src/controllers/ColorController.js diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index a42bef49b8..3c86ca8421 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -88,7 +88,7 @@ { "property": "fill", "glyph": "X", - "control": "textfield", + "control": "color", "inclusive": true } ] diff --git a/platform/forms/bundle.json b/platform/forms/bundle.json index eb63e22432..f7a3c33ae4 100644 --- a/platform/forms/bundle.json +++ b/platform/forms/bundle.json @@ -60,6 +60,10 @@ { "key": "CompositeController", "implementation": "controllers/CompositeController.js" + }, + { + "key": "ColorController", + "implementation": "controllers/ColorController.js" } ] } diff --git a/platform/forms/res/templates/controls/color.html b/platform/forms/res/templates/controls/color.html index 4e44c8e84f..f880838a0c 100644 --- a/platform/forms/res/templates/controls/color.html +++ b/platform/forms/res/templates/controls/color.html @@ -1,3 +1,36 @@ - +
+ + + {{structure.glyph}} + + {{structure.text}} + + + v + + + +
+
+
+ {{ngModel[field] === 'transparent' ? 'x' : '' }} +
+ None +
+
+
+ {{ngModel[field] === color ? 'x' : '' }} +
+
+ +
+
\ No newline at end of file diff --git a/platform/forms/src/controllers/ColorController.js b/platform/forms/src/controllers/ColorController.js new file mode 100644 index 0000000000..6a8c88225a --- /dev/null +++ b/platform/forms/src/controllers/ColorController.js @@ -0,0 +1,81 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + var BASE_COLORS = [ + [ 136, 32, 32 ], + [ 224, 64, 64 ], + [ 240, 160, 72 ], + [ 255, 248, 96 ], + [ 128, 240, 72 ], + [ 128, 248, 248 ], + [ 88, 144, 224 ], + [ 0, 72, 240 ], + [ 136, 80, 240 ], + [ 224, 96, 248 ] + ], + GRADIENTS = [0.75, 0.50, 0.25, -0.25, -0.50, -0.75], + GROUPS = []; + + function toWebColor(triplet) { + return '#' + triplet.map(function (v) { + return (v < 16 ? '0' : '') + v.toString(16); + }).join(''); + } + + function toGradient(triplet, value) { + return triplet.map(function (v) { + return Math.round(value > 0 ? + (v + (255 - v) * value) : + (v * (1 + value)) + ); + }); + } + + function initializeGroups() { + var i, group; + + // Ten grayscale colors + group = []; + while (group.length < 10) { + group.push(toWebColor([ + Math.round(28.3333 * group.length), + Math.round(28.3333 * group.length), + Math.round(28.3333 * group.length) + ])); + } + GROUPS.push(group); + + // Ten basic colors + GROUPS.push(BASE_COLORS.map(toWebColor)); + + // ...and some gradients of those colors + group = []; + GRADIENTS.forEach(function (v) { + group = group.concat(BASE_COLORS.map(function (c) { + return toWebColor(toGradient(c, v)); + })); + }); + GROUPS.push(group); + } + + + + function ColorController() { + if (GROUPS.length === 0) { + initializeGroups(); + } + + return { + groups: function () { + return GROUPS; + } + }; + } + + return ColorController; + } +); \ No newline at end of file From 6068f0f9c28126cb8525127bc3e0f6374d665374 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 17:06:27 -0800 Subject: [PATCH 08/21] [Forms] Add ColorController spec Add spec for ColorController, introduced to support color selection in the Fixed Position toolbar, WTD-881 --- .../test/controllers/ColorControllerSpec.js | 51 +++++++++++++++++++ platform/forms/test/suite.json | 1 + 2 files changed, 52 insertions(+) create mode 100644 platform/forms/test/controllers/ColorControllerSpec.js diff --git a/platform/forms/test/controllers/ColorControllerSpec.js b/platform/forms/test/controllers/ColorControllerSpec.js new file mode 100644 index 0000000000..c8d432d91b --- /dev/null +++ b/platform/forms/test/controllers/ColorControllerSpec.js @@ -0,0 +1,51 @@ +/*global define,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../../src/controllers/ColorController"], + function (ColorController) { + "use strict"; + + var COLOR_REGEX = /^#[0-9a-fA-F]{6}$/; + + describe("The color picker's controller", function () { + var controller; + + beforeEach(function () { + controller = new ColorController(); + }); + + it("exposes groups of colors", function () { + var groups = controller.groups(); + + // Make sure that the groups array is non-empty + expect(Array.isArray(groups)).toBeTruthy(); + expect(groups.length).not.toEqual(0); + + groups.forEach(function (group) { + // Make sure each group is a non-empty array + expect(Array.isArray(group)).toBeTruthy(); + expect(group.length).not.toEqual(0); + // Make sure they're valid web colors + group.forEach(function (color) { + expect(COLOR_REGEX.test(color)).toBeTruthy(); + }); + }); + }); + + it("exposes unique colors", function () { + var count = 0, set = {}; + + // Count each color, and add them to the set + controller.groups().forEach(function (group) { + group.forEach(function (color) { + count += 1; + set[color] = true; + }); + }) + + // Size of set should be number of colors if all were unique + expect(Object.keys(set).length).toEqual(count); + }); + }); + } +); \ No newline at end of file diff --git a/platform/forms/test/suite.json b/platform/forms/test/suite.json index 61eff300e6..ad1c50e39e 100644 --- a/platform/forms/test/suite.json +++ b/platform/forms/test/suite.json @@ -1,6 +1,7 @@ [ "MCTControl", "MCTForm", + "controllers/ColorController", "controllers/CompositeController", "controllers/DateTimeController", "controllers/FormController" From 13793e221e37d0385b4c8ede71da71d3ca997ed2 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 18:05:37 -0800 Subject: [PATCH 09/21] [Fixed Position] Add stroke property Add stroke property to Fixed Position toolbar for borders, line color. WTD-881. --- platform/features/layout/bundle.json | 10 ++++++---- .../layout/res/templates/elements/box.html | 2 +- .../layout/res/templates/elements/image.html | 4 ++-- .../layout/res/templates/elements/line.html | 2 +- .../layout/res/templates/elements/telemetry.html | 2 +- .../layout/res/templates/elements/text.html | 2 +- platform/features/layout/src/FixedController.js | 1 + .../features/layout/src/elements/ElementFactory.js | 14 +++++++++----- .../features/layout/src/elements/ElementProxy.js | 8 ++++---- 9 files changed, 26 insertions(+), 19 deletions(-) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index 3c86ca8421..a85ce3d980 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -29,7 +29,6 @@ "glyph": "+", "control": "menu-button", "text": "Add", - "inclusive": true, "options": [ { "name": "Box", @@ -61,7 +60,6 @@ "method": "order", "glyph": "o", "control": "menu-button", - "inclusive": true, "options": [ { "name": "Move to Top", @@ -88,8 +86,12 @@ { "property": "fill", "glyph": "X", - "control": "color", - "inclusive": true + "control": "color" + }, + { + "property": "stroke", + "glyph": "-", + "control": "color" } ] }, diff --git a/platform/features/layout/res/templates/elements/box.html b/platform/features/layout/res/templates/elements/box.html index 58ecf860d7..3ca8eef7da 100644 --- a/platform/features/layout/res/templates/elements/box.html +++ b/platform/features/layout/res/templates/elements/box.html @@ -1,3 +1,3 @@ -
\ No newline at end of file diff --git a/platform/features/layout/res/templates/elements/image.html b/platform/features/layout/res/templates/elements/image.html index 041147df27..6115b8f4cd 100644 --- a/platform/features/layout/res/templates/elements/image.html +++ b/platform/features/layout/res/templates/elements/image.html @@ -1,3 +1,3 @@ -
-
\ No newline at end of file +
diff --git a/platform/features/layout/res/templates/elements/line.html b/platform/features/layout/res/templates/elements/line.html index 524cd451ca..11408bbe9b 100644 --- a/platform/features/layout/res/templates/elements/line.html +++ b/platform/features/layout/res/templates/elements/line.html @@ -4,7 +4,7 @@ ng-attr-y1="{{parameters.gridSize[1] * ngModel.y1() + 1}}" ng-attr-x2="{{parameters.gridSize[0] * ngModel.x2() + 1}}" ng-attr-y2="{{parameters.gridSize[1] * ngModel.y2() + 1}}" - stroke="lightgray" + ng-attr-stroke="{{ngModel.stroke()}}" stroke-width="2"> \ No newline at end of file diff --git a/platform/features/layout/res/templates/elements/telemetry.html b/platform/features/layout/res/templates/elements/telemetry.html index f5b51a0311..051c33e159 100644 --- a/platform/features/layout/res/templates/elements/telemetry.html +++ b/platform/features/layout/res/templates/elements/telemetry.html @@ -1,4 +1,4 @@ -
{{ngModel.name}} diff --git a/platform/features/layout/res/templates/elements/text.html b/platform/features/layout/res/templates/elements/text.html index 2b52f04598..043df913cf 100644 --- a/platform/features/layout/res/templates/elements/text.html +++ b/platform/features/layout/res/templates/elements/text.html @@ -1,4 +1,4 @@ -
{{ngModel.element.text}}
\ No newline at end of file diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index 442f966033..dc298a1d89 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -193,6 +193,7 @@ define( x: Math.floor(position.x / gridSize[0]), y: Math.floor(position.y / gridSize[1]), id: id, + stroke: "transparent", width: DEFAULT_DIMENSIONS[0], height: DEFAULT_DIMENSIONS[1] }); diff --git a/platform/features/layout/src/elements/ElementFactory.js b/platform/features/layout/src/elements/ElementFactory.js index 4ad229e30a..e61b0136d5 100644 --- a/platform/features/layout/src/elements/ElementFactory.js +++ b/platform/features/layout/src/elements/ElementFactory.js @@ -6,20 +6,24 @@ define( "use strict"; var INITIAL_STATES = { - "fixed.image": {}, + "fixed.image": { + stroke: "transparent" + }, "fixed.box": { - fill: "#888", - border: "transparent" + fill: "#717171", + border: "transparent", + stroke: "transparent" }, "fixed.line": { x: 5, y: 9, x2: 6, - y2: 6 + y2: 6, + stroke: "#717171" }, "fixed.text": { fill: "transparent", - border: "transparent" + stroke: "transparent" } }, DIALOGS = { diff --git a/platform/features/layout/src/elements/ElementProxy.js b/platform/features/layout/src/elements/ElementProxy.js index 438725f384..7d5fa3540b 100644 --- a/platform/features/layout/src/elements/ElementProxy.js +++ b/platform/features/layout/src/elements/ElementProxy.js @@ -49,11 +49,11 @@ define( */ y: new AccessorMutator(element, 'y'), /** - * Get and/or set the z index of this element. - * @param {number} [z] the new z index (if setting) - * @returns {number} the z index + * Get and/or set the stroke color of this element. + * @param {string} [stroke] the new stroke color (if setting) + * @returns {string} the stroke color */ - z: new AccessorMutator(element, 'z'), + stroke: new AccessorMutator(element, 'stroke'), /** * Get and/or set the width of this element. * Units are in fixed position grid space. From 894a5b8f8945a5a76e5125d0e8bb464506f5368b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 18:17:24 -0800 Subject: [PATCH 10/21] [Fixed Position] Add text color property Add a property to the Fixed Position toolbar to set text color, WTD-881. --- platform/features/layout/bundle.json | 6 ++++++ .../features/layout/res/templates/elements/telemetry.html | 2 +- platform/features/layout/res/templates/elements/text.html | 2 +- platform/features/layout/src/FixedController.js | 1 + platform/features/layout/src/elements/ElementFactory.js | 3 ++- platform/features/layout/src/elements/TextProxy.js | 7 +++++++ platform/forms/res/templates/controls/color.html | 2 +- 7 files changed, 19 insertions(+), 4 deletions(-) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index a85ce3d980..ba27f5e1c9 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -92,6 +92,12 @@ "property": "stroke", "glyph": "-", "control": "color" + }, + { + "property": "color", + "glyph": "\u1D1B", + "mandatory": true, + "control": "color" } ] }, diff --git a/platform/features/layout/res/templates/elements/telemetry.html b/platform/features/layout/res/templates/elements/telemetry.html index 051c33e159..8f8f44461f 100644 --- a/platform/features/layout/res/templates/elements/telemetry.html +++ b/platform/features/layout/res/templates/elements/telemetry.html @@ -1,4 +1,4 @@ -
{{ngModel.name}} diff --git a/platform/features/layout/res/templates/elements/text.html b/platform/features/layout/res/templates/elements/text.html index 043df913cf..627468d3e2 100644 --- a/platform/features/layout/res/templates/elements/text.html +++ b/platform/features/layout/res/templates/elements/text.html @@ -1,4 +1,4 @@ -
{{ngModel.element.text}}
\ No newline at end of file diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index dc298a1d89..963afc8713 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -194,6 +194,7 @@ define( y: Math.floor(position.y / gridSize[1]), id: id, stroke: "transparent", + color: "#717171", width: DEFAULT_DIMENSIONS[0], height: DEFAULT_DIMENSIONS[1] }); diff --git a/platform/features/layout/src/elements/ElementFactory.js b/platform/features/layout/src/elements/ElementFactory.js index e61b0136d5..3ae8cad80a 100644 --- a/platform/features/layout/src/elements/ElementFactory.js +++ b/platform/features/layout/src/elements/ElementFactory.js @@ -23,7 +23,8 @@ define( }, "fixed.text": { fill: "transparent", - stroke: "transparent" + stroke: "transparent", + color: "#717171" } }, DIALOGS = { diff --git a/platform/features/layout/src/elements/TextProxy.js b/platform/features/layout/src/elements/TextProxy.js index 53ba553672..a1da84399c 100644 --- a/platform/features/layout/src/elements/TextProxy.js +++ b/platform/features/layout/src/elements/TextProxy.js @@ -20,6 +20,13 @@ define( function TextProxy(element, index, elements) { var proxy = new BoxProxy(element, index, elements); + /** + * Get and/or set the text color of this element. + * @param {string} [color] the new text color (if setting) + * @returns {string} the text color + */ + proxy.color = new AccessorMutator(element, 'color'); + return proxy; } diff --git a/platform/forms/res/templates/controls/color.html b/platform/forms/res/templates/controls/color.html index f880838a0c..31f9f3bec2 100644 --- a/platform/forms/res/templates/controls/color.html +++ b/platform/forms/res/templates/controls/color.html @@ -15,7 +15,7 @@
-
+
{{ngModel[field] === 'transparent' ? 'x' : '' }} From 0b2cd524339378e45ef10d190c56fddaf4725d72 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 18:28:30 -0800 Subject: [PATCH 11/21] [Fixed Position] Add image proxy Add selection proxy for image elements to support image URL dialog, WTD-881. --- platform/features/layout/bundle.json | 22 +++++++++++ .../layout/src/elements/ImageProxy.js | 35 ++++++++++++++++++ .../features/layout/src/elements/TextProxy.js | 7 ++++ .../layout/test/elements/ImageProxySpec.js | 37 +++++++++++++++++++ 4 files changed, 101 insertions(+) create mode 100644 platform/features/layout/src/elements/ImageProxy.js create mode 100644 platform/features/layout/test/elements/ImageProxySpec.js diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index ba27f5e1c9..dabe340036 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -98,6 +98,28 @@ "glyph": "\u1D1B", "mandatory": true, "control": "color" + }, + { + "property": "url", + "glyph": "", + "control": "dialog", + "title": "Image Properties", + "dialog": { + "control": "textfield", + "name": "Image URL", + "required": true + } + }, + { + "property": "text", + "glyph": "", + "control": "dialog", + "title": "Text Properties", + "dialog": { + "control": "textfield", + "name": "Text", + "required": true + } } ] }, diff --git a/platform/features/layout/src/elements/ImageProxy.js b/platform/features/layout/src/elements/ImageProxy.js new file mode 100644 index 0000000000..56810eb4ea --- /dev/null +++ b/platform/features/layout/src/elements/ImageProxy.js @@ -0,0 +1,35 @@ +/*global define*/ + +define( + ['./ElementProxy', './AccessorMutator'], + function (ElementProxy, AccessorMutator) { + 'use strict'; + + /** + * Selection proxy for Image elements in a fixed position view. + * + * Note that arguments here are meant to match those expected + * by `Array.prototype.map` + * + * @constructor + * @param element the fixed position element, as stored in its + * configuration + * @param index the element's index within its array + * @param {Array} elements the full array of elements + */ + function ImageProxy(element, index, elements) { + var proxy = new ElementProxy(element, index, elements); + + /** + * Get and/or set the displayed text of this element. + * @param {string} [text] the new text (if setting) + * @returns {string} the text + */ + proxy.url = new AccessorMutator(element, 'url'); + + return proxy; + } + + return ImageProxy; + } +); \ No newline at end of file diff --git a/platform/features/layout/src/elements/TextProxy.js b/platform/features/layout/src/elements/TextProxy.js index a1da84399c..ebc3413b79 100644 --- a/platform/features/layout/src/elements/TextProxy.js +++ b/platform/features/layout/src/elements/TextProxy.js @@ -27,6 +27,13 @@ define( */ proxy.color = new AccessorMutator(element, 'color'); + /** + * Get and/or set the displayed text of this element. + * @param {string} [text] the new text (if setting) + * @returns {string} the text + */ + proxy.text = new AccessorMutator(element, 'text'); + return proxy; } diff --git a/platform/features/layout/test/elements/ImageProxySpec.js b/platform/features/layout/test/elements/ImageProxySpec.js new file mode 100644 index 0000000000..6d74759a0e --- /dev/null +++ b/platform/features/layout/test/elements/ImageProxySpec.js @@ -0,0 +1,37 @@ +/*global define,describe,it,expect,beforeEach,jasmine*/ + +define( + ['../../src/elements/ImageProxy'], + function (ImageProxy) { + "use strict"; + + describe("A fixed position image proxy", function () { + var testElement, + testElements, + proxy; + + beforeEach(function () { + testElement = { + x: 1, + y: 2, + width: 42, + height: 24, + url: "http://www.nasa.gov" + }; + testElements = [ {}, {}, testElement, {} ]; + proxy = new ImageProxy( + testElement, + testElements.indexOf(testElement), + testElements + ); + }); + + it("provides getter/setter for image URL", function () { + expect(proxy.url()).toEqual("http://www.nasa.gov"); + expect(proxy.url("http://www.nasa.gov/some.jpg")) + .toEqual("http://www.nasa.gov/some.jpg"); + expect(proxy.url()).toEqual("http://www.nasa.gov/some.jpg"); + }); + }); + } +); From 3e95a96c69c312aa351c1a908bfb99b00222ede7 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 18:52:53 -0800 Subject: [PATCH 12/21] [Forms] Add dialog button spec Add spec for a button which should show a dialog for input on a single value, to support Image properties in Fixed Position view, WTD-881. --- .../forms/res/templates/controls/dialog.html | 4 + .../src/controllers/DialogButtonController.js | 0 .../controllers/DialogButtonControllerSpec.js | 116 ++++++++++++++++++ platform/forms/test/suite.json | 1 + 4 files changed, 121 insertions(+) create mode 100644 platform/forms/res/templates/controls/dialog.html create mode 100644 platform/forms/src/controllers/DialogButtonController.js create mode 100644 platform/forms/test/controllers/DialogButtonControllerSpec.js diff --git a/platform/forms/res/templates/controls/dialog.html b/platform/forms/res/templates/controls/dialog.html new file mode 100644 index 0000000000..05b0d39d43 --- /dev/null +++ b/platform/forms/res/templates/controls/dialog.html @@ -0,0 +1,4 @@ + + \ No newline at end of file diff --git a/platform/forms/src/controllers/DialogButtonController.js b/platform/forms/src/controllers/DialogButtonController.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/platform/forms/test/controllers/DialogButtonControllerSpec.js b/platform/forms/test/controllers/DialogButtonControllerSpec.js new file mode 100644 index 0000000000..a67ccdcbd1 --- /dev/null +++ b/platform/forms/test/controllers/DialogButtonControllerSpec.js @@ -0,0 +1,116 @@ +/*global define,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../../src/controllers/DialogButtonController"], + function (DialogButtonController) { + "use strict"; + + describe("A dialog button controller", function () { + var mockScope, + mockDialogService, + mockPromise, + testStructure, + controller; + + beforeEach(function () { + mockScope = jasmine.createSpyObj( + '$scope', + [ '$watch' ] + ); + mockDialogService = jasmine.createSpyObj( + 'dialogService', + [ 'getUserInput' ] + ); + mockPromise = jasmine.createSpyObj( + 'promise', + [ 'then' ] + ); + testStructure = { + key: "testKey", + name: "A Test", + glyph: "T", + description: "Test description", + control: "dialog-button", + title: "Test title", + dialog: { + "control": "textfield", + "name": "Inner control" + } + }; + + mockScope.ngModel = { testKey: "initial test value" }; + mockScope.structure = testStructure; + + mockDialogService.getUserInput.andReturn(mockPromise); + + controller = new DialogButtonController( + mockScope, + mockDialogService + ); + }); + + it("provides a structure for a button control", function () { + var buttonStructure; + + // Template is just a mct-control pointing to a button + // control, so this controller needs to set up all the + // logic for showing a dialog and collecting user input + // when that button gets clicked. + expect(mockScope.$watch).toHaveBeenCalledWith( + "structure", // As passed in via mct-control + jasmine.any(Function) + ); + + mockScope.$watch.mostRecentCall.args(testStructure); + + buttonStructure = controller.getButtonStructure(); + expect(buttonStructure.glyph).toEqual(testStructure.glyph); + expect(buttonStructure.description).toEqual(testStructure.description); + expect(buttonStructure.name).toEqual(testStructure.name); + expect(buttonStructure.click).toEqual(jasmine.any(Function)); + }); + + it("shows a dialog when clicked", function () { + mockScope.$watch.mostRecentCall.args(testStructure); + // Verify precondition - no dialog shown + expect(mockDialogService.getUserInput).not.toHaveBeenCalled(); + // Click! + controller.getButtonStructure().click(); + // Should have shown a dialog + expect(mockDialogService.getUserInput).toHaveBeenCalled(); + }); + + it("stores user input to the model", function () { + var key, input = {}; + // Show dialog, click... + mockScope.$watch.mostRecentCall.args(testStructure); + controller.getButtonStructure().click(); + // Should be listening to 'then' + expect(mockPromise.then) + .toHaveBeenCalledWith(jasmine.any(Function)); + // Find the key that the dialog should return + key = mockDialogService.getUserInput.mostRecentCall + .args[0].sections[0].rows[0].key; + // Provide 'user input' + input[key] = "test user input"; + // Resolve the promise with it + mockPromise.then.mostRecentCall.args[0](input); + // ... should have been placed into the model + expect(mockScope.ngModel.testKey).toEqual("test user input"); + }); + + it("supplies initial model state to the dialog", function () { + var key, state; + mockScope.$watch.mostRecentCall.args(testStructure); + controller.getButtonStructure().click(); + // Find the key that the dialog should return + key = mockDialogService.getUserInput.mostRecentCall + .args[0].sections[0].rows[0].key; + // Get the initial state provided to the dialog + state = mockDialogService.getUserInput.mostRecentCall.args[1]; + // Should have had value from ngModel stored to that key + expect(state[key]).toEqual("initial test value"); + }); + }); + } +); \ No newline at end of file diff --git a/platform/forms/test/suite.json b/platform/forms/test/suite.json index ad1c50e39e..be4d6151db 100644 --- a/platform/forms/test/suite.json +++ b/platform/forms/test/suite.json @@ -4,5 +4,6 @@ "controllers/ColorController", "controllers/CompositeController", "controllers/DateTimeController", + "controllers/DialogButtonController", "controllers/FormController" ] \ No newline at end of file From 0030efc29e6b8ad47828fc700fb1f4875275831d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 19:06:09 -0800 Subject: [PATCH 13/21] [Forms] Implement DialogButtonController Implement controller for dialog buttons sufficient to satisfy spec; these will be used to launch dialogs to collect user input for Image Properties et al to complete toolbar for Fixed Position view, WTD-881. --- .../src/controllers/DialogButtonController.js | 72 +++++++++++++++++++ .../controllers/DialogButtonControllerSpec.js | 10 +-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/platform/forms/src/controllers/DialogButtonController.js b/platform/forms/src/controllers/DialogButtonController.js index e69de29bb2..809ba58f45 100644 --- a/platform/forms/src/controllers/DialogButtonController.js +++ b/platform/forms/src/controllers/DialogButtonController.js @@ -0,0 +1,72 @@ +/*global define*/ +define( + [], + function () { + 'use strict'; + + /** + * Controller for the `dialog-button` control type. Provides + * structure for a button (embedded via the template) which + * will show a dialog for editing a single property when clicked. + * @constructor + * @param $scope the control's Angular scope + * @param {DialogService} dialogService service to use to prompt + * for user input + */ + function DialogButtonController($scope, dialogService) { + var buttonStructure, + buttonForm, + field; + + // Store the result of user input to the model + function storeResult(result) { + $scope.ngModel[$scope.field] = result[$scope.field]; + } + + // Prompt for user input + function showDialog() { + dialogService.getUserInput(buttonForm, $scope.ngModel) + .then(storeResult); + } + + // Refresh state based on structure for this control + function refreshStructure(structure) { + var row = Object.create(structure.dialog || {}); + + structure = structure || {}; + + // Add the key, to read back from that row + row.key = $scope.field; + + // Prepare the structure for the button itself + buttonStructure = {}; + buttonStructure.glyph = structure.glyph; + buttonStructure.name = structure.name; + buttonStructure.description = structure.description; + buttonStructure.click = showDialog; + + // Prepare the form; a single row + buttonForm = { + name: structure.title, + sections: [ { rows: [ row ] } ] + }; + } + + $scope.$watch('structure', refreshStructure); + + return { + /** + * Get the structure for an `mct-control` of type + * `button`; a dialog will be launched when this button + * is clicked. + * @returns dialog structure + */ + getButtonStructure: function () { + return buttonStructure; + } + }; + } + + return DialogButtonController; + } +); \ No newline at end of file diff --git a/platform/forms/test/controllers/DialogButtonControllerSpec.js b/platform/forms/test/controllers/DialogButtonControllerSpec.js index a67ccdcbd1..b8914d0bae 100644 --- a/platform/forms/test/controllers/DialogButtonControllerSpec.js +++ b/platform/forms/test/controllers/DialogButtonControllerSpec.js @@ -26,7 +26,6 @@ define( [ 'then' ] ); testStructure = { - key: "testKey", name: "A Test", glyph: "T", description: "Test description", @@ -38,6 +37,7 @@ define( } }; + mockScope.field = "testKey"; mockScope.ngModel = { testKey: "initial test value" }; mockScope.structure = testStructure; @@ -61,7 +61,7 @@ define( jasmine.any(Function) ); - mockScope.$watch.mostRecentCall.args(testStructure); + mockScope.$watch.mostRecentCall.args[1](testStructure); buttonStructure = controller.getButtonStructure(); expect(buttonStructure.glyph).toEqual(testStructure.glyph); @@ -71,7 +71,7 @@ define( }); it("shows a dialog when clicked", function () { - mockScope.$watch.mostRecentCall.args(testStructure); + mockScope.$watch.mostRecentCall.args[1](testStructure); // Verify precondition - no dialog shown expect(mockDialogService.getUserInput).not.toHaveBeenCalled(); // Click! @@ -83,7 +83,7 @@ define( it("stores user input to the model", function () { var key, input = {}; // Show dialog, click... - mockScope.$watch.mostRecentCall.args(testStructure); + mockScope.$watch.mostRecentCall.args[1](testStructure); controller.getButtonStructure().click(); // Should be listening to 'then' expect(mockPromise.then) @@ -101,7 +101,7 @@ define( it("supplies initial model state to the dialog", function () { var key, state; - mockScope.$watch.mostRecentCall.args(testStructure); + mockScope.$watch.mostRecentCall.args[1](testStructure); controller.getButtonStructure().click(); // Find the key that the dialog should return key = mockDialogService.getUserInput.mostRecentCall From 1c69d1f2da40e3e17b952af0c9d05bedc6382830 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 19:16:09 -0800 Subject: [PATCH 14/21] [Fixed Position] Add text, image properties Add buttons for text, image properties to the Fixed Position toolbar, WTD-881. --- platform/features/layout/bundle.json | 8 ++++---- platform/features/layout/src/elements/ElementProxies.js | 6 +++--- platform/forms/bundle.json | 9 +++++++++ platform/forms/res/templates/controls/dialog.html | 9 +++++---- platform/forms/src/controllers/DialogButtonController.js | 8 ++++++-- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index dabe340036..3a3cf86c60 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -101,8 +101,8 @@ }, { "property": "url", - "glyph": "", - "control": "dialog", + "glyph": "\u2353", + "control": "dialog-button", "title": "Image Properties", "dialog": { "control": "textfield", @@ -112,8 +112,8 @@ }, { "property": "text", - "glyph": "", - "control": "dialog", + "glyph": "p", + "control": "dialog-button", "title": "Text Properties", "dialog": { "control": "textfield", diff --git a/platform/features/layout/src/elements/ElementProxies.js b/platform/features/layout/src/elements/ElementProxies.js index 125d0c7f10..8c828cbb83 100644 --- a/platform/features/layout/src/elements/ElementProxies.js +++ b/platform/features/layout/src/elements/ElementProxies.js @@ -1,15 +1,15 @@ /*global define*/ define( - ['./TelemetryProxy', './ElementProxy', './LineProxy', './BoxProxy', './TextProxy'], - function (TelemetryProxy, ElementProxy, LineProxy, BoxProxy, TextProxy) { + ['./TelemetryProxy', './ImageProxy', './LineProxy', './BoxProxy', './TextProxy'], + function (TelemetryProxy, ImageProxy, LineProxy, BoxProxy, TextProxy) { "use strict"; return { "fixed.telemetry": TelemetryProxy, "fixed.line": LineProxy, "fixed.box": BoxProxy, - "fixed.image": ElementProxy, + "fixed.image": ImageProxy, "fixed.text": TextProxy }; } diff --git a/platform/forms/bundle.json b/platform/forms/bundle.json index f7a3c33ae4..c5d064d81f 100644 --- a/platform/forms/bundle.json +++ b/platform/forms/bundle.json @@ -49,6 +49,10 @@ { "key": "menu-button", "templateUrl": "templates/controls/menu-button.html" + }, + { + "key": "dialog-button", + "templateUrl": "templates/controls/dialog.html" } ], "controllers": [ @@ -64,6 +68,11 @@ { "key": "ColorController", "implementation": "controllers/ColorController.js" + }, + { + "key": "DialogButtonController", + "implementation": "controllers/DialogButtonController.js", + "depends": [ "$scope", "dialogService" ] } ] } diff --git a/platform/forms/res/templates/controls/dialog.html b/platform/forms/res/templates/controls/dialog.html index 05b0d39d43..d32c29807f 100644 --- a/platform/forms/res/templates/controls/dialog.html +++ b/platform/forms/res/templates/controls/dialog.html @@ -1,4 +1,5 @@ - - \ No newline at end of file + + + + \ No newline at end of file diff --git a/platform/forms/src/controllers/DialogButtonController.js b/platform/forms/src/controllers/DialogButtonController.js index 809ba58f45..622122e6d8 100644 --- a/platform/forms/src/controllers/DialogButtonController.js +++ b/platform/forms/src/controllers/DialogButtonController.js @@ -25,8 +25,12 @@ define( // Prompt for user input function showDialog() { - dialogService.getUserInput(buttonForm, $scope.ngModel) - .then(storeResult); + // Prepare initial state + var state = {}; + state[$scope.field] = $scope.ngModel[$scope.field]; + + // Show dialog, then store user input (if any) + dialogService.getUserInput(buttonForm, state).then(storeResult); } // Refresh state based on structure for this control From 41a013d1987c4365eb61a5f2d76a78b7538fee62 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 19:25:33 -0800 Subject: [PATCH 15/21] [Fixed Position] Add button to show/hide telemetry name Add button to show/hide name of telemetry elements in fixed position view, WTD-881. --- platform/features/layout/bundle.json | 12 +++++++++++ .../res/templates/elements/telemetry.html | 3 ++- .../features/layout/src/FixedController.js | 1 + .../layout/src/elements/TelemetryProxy.js | 21 +++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index 3a3cf86c60..cd7426c04c 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -120,6 +120,18 @@ "name": "Text", "required": true } + }, + { + "method": "showTitle", + "glyph": "+", + "control": "button", + "description": "Show telemetry element title." + }, + { + "method": "hideTitle", + "glyph": "X", + "control": "button", + "description": "Hide telemetry element title." } ] }, diff --git a/platform/features/layout/res/templates/elements/telemetry.html b/platform/features/layout/res/templates/elements/telemetry.html index 8f8f44461f..3a40d416f8 100644 --- a/platform/features/layout/res/templates/elements/telemetry.html +++ b/platform/features/layout/res/templates/elements/telemetry.html @@ -1,6 +1,7 @@
-
+
{{ngModel.name}}
diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index 963afc8713..f795093dd4 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -195,6 +195,7 @@ define( id: id, stroke: "transparent", color: "#717171", + titled: true, width: DEFAULT_DIMENSIONS[0], height: DEFAULT_DIMENSIONS[1] }); diff --git a/platform/features/layout/src/elements/TelemetryProxy.js b/platform/features/layout/src/elements/TelemetryProxy.js index b587465df4..c358a4a480 100644 --- a/platform/features/layout/src/elements/TelemetryProxy.js +++ b/platform/features/layout/src/elements/TelemetryProxy.js @@ -5,6 +5,9 @@ define( function (TextProxy, AccessorMutator) { 'use strict'; + // Method names to expose from this proxy + var HIDE = 'hideTitle', SHOW = 'showTitle'; + /** * Selection proxy for telemetry elements in a fixed position view. * @@ -20,9 +23,27 @@ define( function TelemetryProxy(element, index, elements) { var proxy = new TextProxy(element, index, elements); + // Toggle the visibility of the title + function toggle() { + // Toggle the state + element.titled = !element.titled; + + // Change which method is exposed, to influence + // which button is shown in the toolbar + delete proxy[SHOW]; + delete proxy[HIDE]; + proxy[element.titled ? HIDE : SHOW] = toggle; + } + // Expose the domain object identifier proxy.id = element.id; + // Expose initial toggle + proxy[element.titled ? HIDE : SHOW] = toggle; + + // Don't expose text configuration + delete proxy.text; + return proxy; } From 5ee0f9372b520e4c012af91b6d9a77528f70544a Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 19:27:47 -0800 Subject: [PATCH 16/21] [Fixed Position] Fix appearance of Remove button Update appearance of Remove button in Fixed Position view to match UI docs, WTD-881. --- platform/features/layout/bundle.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/platform/features/layout/bundle.json b/platform/features/layout/bundle.json index cd7426c04c..399399ce80 100644 --- a/platform/features/layout/bundle.json +++ b/platform/features/layout/bundle.json @@ -140,8 +140,7 @@ { "method": "remove", "control": "button", - "text": "Remove", - "inclusive": true + "glyph": "Z" } ] } From 307fbe21518506dcc038e2f936664efb618a27d1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 23 Feb 2015 19:47:56 -0800 Subject: [PATCH 17/21] [Edit] Update EditToolbar spec Update EditToolbar spec to reflect changes for WTD-881. --- .../edit/src/representers/EditToolbar.js | 12 +- .../edit/test/representers/EditToolbarSpec.js | 127 ++++++++---------- 2 files changed, 58 insertions(+), 81 deletions(-) diff --git a/platform/commonUI/edit/src/representers/EditToolbar.js b/platform/commonUI/edit/src/representers/EditToolbar.js index 0fd916d561..522a808aaf 100644 --- a/platform/commonUI/edit/src/representers/EditToolbar.js +++ b/platform/commonUI/edit/src/representers/EditToolbar.js @@ -16,7 +16,6 @@ define( * the current selection. * * @param structure toolbar structure, as provided by view definition - * @param {Array} selection the current selection state * @param {Function} commit callback to invoke after changes * @constructor */ @@ -109,7 +108,7 @@ define( function isApplicable(item) { var property = (item || {}).property, method = (item || {}).method, - exclusive = (item || {}).exclusive; + exclusive = !!(item || {}).exclusive; // Check if a selected item defines this property function hasProperty(selected) { @@ -151,11 +150,6 @@ define( return converted; } - // Used to filter out sections that have become empty - function nonEmpty(section) { - return section && section.items && section.items.length > 0; - } - // Prepare a toolbar section function convertSection(section) { var converted = Object.create(section || {}); @@ -188,9 +182,7 @@ define( } toolbarStructure.sections = - ((structure || {}).sections || []) - .map(convertSection) - .filter(nonEmpty); + ((structure || {}).sections || []).map(convertSection); toolbarState = []; diff --git a/platform/commonUI/edit/test/representers/EditToolbarSpec.js b/platform/commonUI/edit/test/representers/EditToolbarSpec.js index 6f93bf04e8..ea2fa928f6 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarSpec.js @@ -6,34 +6,41 @@ define( "use strict"; describe("An Edit mode toolbar", function () { - var testStructure, + var mockCommit, + testStructure, testAB, testABC, testABC2, testABCXYZ, testABCYZ, - testM; + testM, + toolbar; + + function getVisibility(obj) { + return !obj.hidden; + } beforeEach(function () { + mockCommit = jasmine.createSpy('commit'); testStructure = { sections: [ { items: [ - { name: "A", property: "a" }, - { name: "B", property: "b" }, - { name: "C", property: "c" } + { name: "A", property: "a", exclusive: true }, + { name: "B", property: "b", exclusive: true }, + { name: "C", property: "c", exclusive: true } ] }, { items: [ - { name: "X", property: "x", inclusive: true }, - { name: "Y", property: "y" }, - { name: "Z", property: "z" } + { name: "X", property: "x" }, + { name: "Y", property: "y", exclusive: true }, + { name: "Z", property: "z", exclusive: true } ] }, { items: [ - { name: "M", method: "m" } + { name: "M", method: "m", exclusive: true } ] } ] @@ -44,6 +51,8 @@ define( testABCXYZ = { a: 0, b: 1, c: 2, x: 'X!', y: 'Y!', z: 'Z!' }; testABCYZ = { a: 0, b: 1, c: 2, y: 'Y!', z: 'Z!' }; testM = { m: jasmine.createSpy("method") }; + + toolbar = new EditToolbar(testStructure, mockCommit); }); it("provides properties from the original structure", function () { @@ -67,27 +76,25 @@ define( ).not.toBeUndefined(); }); - it("prunes empty sections", function () { + it("marks empty sections as hidden", function () { // Verify that all sections are included when applicable... - expect( - new EditToolbar(testStructure, [ testABCXYZ ]) - .getStructure() - .sections - .length - ).toEqual(2); + toolbar.setSelection([ testABCXYZ ]); + expect(toolbar.getStructure().sections.map(getVisibility)) + .toEqual([ true, true, false ]); + // ...but omitted when only some are applicable - expect( - new EditToolbar(testStructure, [ testABC ]) - .getStructure() - .sections - .length - ).toEqual(1); + toolbar.setSelection([ testABC ]); + expect(toolbar.getStructure().sections.map(getVisibility)) + .toEqual([ true, false, false ]); }); it("reads properties from selections", function () { - var toolbar = new EditToolbar(testStructure, [ testABC ]), - structure = toolbar.getStructure(), - state = toolbar.getState(); + var structure, state; + + toolbar.setSelection([ testABC ]); + + structure = toolbar.getStructure(); + state = toolbar.getState(); expect(state[structure.sections[0].items[0].key]) .toEqual(testABC.a); @@ -98,11 +105,11 @@ define( }); it("reads properties from getters", function () { - var toolbar, structure, state; + var structure, state; testABC.a = function () { return "from a getter!"; }; - toolbar = new EditToolbar(testStructure, [ testABC ]); + toolbar.setSelection([ testABC ]); structure = toolbar.getStructure(); state = toolbar.getState(); @@ -111,10 +118,9 @@ define( }); it("sets properties on update", function () { - var toolbar = new EditToolbar(testStructure, [ testABC ]), - structure = toolbar.getStructure(); + toolbar.setSelection([ testABC ]); toolbar.updateState( - structure.sections[0].items[0].key, + toolbar.getStructure().sections[0].items[0].key, "new value" ); // Should have updated the underlying object @@ -122,11 +128,11 @@ define( }); it("invokes setters on update", function () { - var toolbar, structure, state; + var structure, state; testABC.a = jasmine.createSpy('a'); - toolbar = new EditToolbar(testStructure, [ testABC ]); + toolbar.setSelection([ testABC ]); structure = toolbar.getStructure(); toolbar.updateState( @@ -139,68 +145,47 @@ define( it("removes inapplicable items", function () { // First, verify with all items - expect( - new EditToolbar(testStructure, [ testABC ]) - .getStructure() - .sections[0] - .items - .length - ).toEqual(3); + toolbar.setSelection([ testABC ]); + expect(toolbar.getStructure().sections[0].items.map(getVisibility)) + .toEqual([ true, true, true ]); // Then, try with some items omitted - expect( - new EditToolbar(testStructure, [ testABC, testAB ]) - .getStructure() - .sections[0] - .items - .length - ).toEqual(2); + toolbar.setSelection([ testABC, testAB ]); + expect(toolbar.getStructure().sections[0].items.map(getVisibility)) + .toEqual([ true, true, false ]); }); it("removes inconsistent states", function () { // Only two of three values match among these selections - expect( - new EditToolbar(testStructure, [ testABC, testABC2 ]) - .getStructure() - .sections[0] - .items - .length - ).toEqual(2); + toolbar.setSelection([ testABC, testABC2 ]); + expect(toolbar.getStructure().sections[0].items.map(getVisibility)) + .toEqual([ false, true, true ]); }); it("allows inclusive items", function () { // One inclusive item is in the set, property 'x' of the // second section; make sure items are pruned down // when only some of the selection has x,y,z properties - expect( - new EditToolbar(testStructure, [ testABC, testABCXYZ ]) - .getStructure() - .sections[1] - .items - .length - ).toEqual(1); + toolbar.setSelection([ testABC, testABCXYZ ]); + expect(toolbar.getStructure().sections[1].items.map(getVisibility)) + .toEqual([ true, false, false ]); }); it("removes inclusive items when there are no matches", function () { - expect( - new EditToolbar(testStructure, [ testABCYZ ]) - .getStructure() - .sections[1] - .items - .length - ).toEqual(2); + toolbar.setSelection([ testABCYZ ]); + expect(toolbar.getStructure().sections[1].items.map(getVisibility)) + .toEqual([ false, true, true ]); }); it("adds click functions when a method is specified", function () { - var testCommit = jasmine.createSpy('commit'), - toolbar = new EditToolbar(testStructure, [ testM ], testCommit); + toolbar.setSelection([testM]); // Verify precondition expect(testM.m).not.toHaveBeenCalled(); // Click! - toolbar.getStructure().sections[0].items[0].click(); + toolbar.getStructure().sections[2].items[0].click(); // Should have called the underlying function expect(testM.m).toHaveBeenCalled(); // Should also have committed the change - expect(testCommit).toHaveBeenCalled(); + expect(mockCommit).toHaveBeenCalled(); }); }); } From 8452b2ab129581c1deda8e1b39a68ee33c9a2108 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Feb 2015 09:07:57 -0800 Subject: [PATCH 18/21] [Fixed Position] Update specs Update failing specs after changes for WTD-881. --- .../src/representers/EditToolbarRepresenter.js | 4 ++-- .../representers/EditToolbarRepresenterSpec.js | 15 +++++---------- .../layout/test/elements/ElementProxySpec.js | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index 5f2b740b72..df390617fd 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -64,8 +64,6 @@ define( if (attrs.toolbar) { // Initialize toolbar object toolbar = new EditToolbar(definition, commit); - // Expose toolbar state under that name - scope.$parent[attrs.toolbar] = toolbarObject; } } @@ -96,6 +94,8 @@ define( scope.$watchCollection(getState, updateState); // Watch for changes in the current selection state scope.$watchCollection("selection", updateSelection); + // Expose toolbar state under that name + scope.$parent[attrs.toolbar] = toolbarObject; } return { diff --git a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js index 9ca9bb65d4..aea9ef3b03 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js @@ -35,7 +35,7 @@ define( }); it("exposes toolbar state under a attr-defined name", function () { - // A strucutre/state object should have been added to the + // A structure/state object should have been added to the // parent scope under the name provided in the "toolbar" // attribute expect(mockScope.$parent.testToolbar).toBeDefined(); @@ -48,18 +48,12 @@ define( }); it("watches for toolbar state changes", function () { - expect(mockScope.$parent.$watchCollection).toHaveBeenCalledWith( - "testToolbar.state", + expect(mockScope.$watchCollection).toHaveBeenCalledWith( + jasmine.any(Function), jasmine.any(Function) ); }); - it("stops watching toolbar state when destroyed", function () { - expect(mockUnwatch).not.toHaveBeenCalled(); - representer.destroy(); - expect(mockUnwatch).toHaveBeenCalled(); - }); - // Verify a simple interaction between selection state and toolbar // state; more complicated interactions are tested in EditToolbar. it("conveys state changes", function () { @@ -78,7 +72,8 @@ define( // Update the state mockScope.$parent.testToolbar.state[0] = 456; - mockScope.$parent.$watchCollection.mostRecentCall.args[1]( + // Invoke the first watch (assumed to be for toolbar state) + mockScope.$watchCollection.calls[0].args[1]( mockScope.$parent.testToolbar.state ); diff --git a/platform/features/layout/test/elements/ElementProxySpec.js b/platform/features/layout/test/elements/ElementProxySpec.js index 32e819ace8..ab90204ce0 100644 --- a/platform/features/layout/test/elements/ElementProxySpec.js +++ b/platform/features/layout/test/elements/ElementProxySpec.js @@ -14,7 +14,7 @@ define( testElement = { x: 1, y: 2, - z: 3, + stroke: '#717171', width: 42, height: 24 }; From 59ff539bfb844128d42538e495259a95252ba14c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Feb 2015 09:16:43 -0800 Subject: [PATCH 19/21] [Fixed Position] Update specs for coverage Update specs to restore code coverage after changes for WTD-881. --- .../representers/EditToolbarRepresenter.js | 2 ++ .../EditToolbarRepresenterSpec.js | 12 +++++++++++ .../test/elements/TelemetryProxySpec.js | 21 +++++++++++++++++++ .../test/controllers/ColorControllerSpec.js | 2 +- 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index df390617fd..b17c7f7d88 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -64,6 +64,8 @@ define( if (attrs.toolbar) { // Initialize toolbar object toolbar = new EditToolbar(definition, commit); + // Ensure toolbar state is exposed + scope.$parent[attrs.toolbar] = toolbarObject; } } diff --git a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js index aea9ef3b03..56093b5e86 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js @@ -48,10 +48,22 @@ define( }); it("watches for toolbar state changes", function () { + representer.represent({}); expect(mockScope.$watchCollection).toHaveBeenCalledWith( jasmine.any(Function), jasmine.any(Function) ); + expect(mockScope.$watchCollection.calls[0].args[0]()) + .toBe(mockScope.$parent.testToolbar.state); + }); + + it("removes state from parent scope on destroy", function () { + // Verify precondition + expect(mockScope.$parent.testToolbar).toBeDefined(); + // Destroy the represeter + representer.destroy(); + // Should have removed toolbar state from view + expect(mockScope.$parent.testToolbar).toBeUndefined(); }); // Verify a simple interaction between selection state and toolbar diff --git a/platform/features/layout/test/elements/TelemetryProxySpec.js b/platform/features/layout/test/elements/TelemetryProxySpec.js index fdd7e555d0..2042535c90 100644 --- a/platform/features/layout/test/elements/TelemetryProxySpec.js +++ b/platform/features/layout/test/elements/TelemetryProxySpec.js @@ -30,6 +30,27 @@ define( it("exposes the element's id", function () { expect(proxy.id).toEqual('test-id'); }); + + it("allows title to be shown/hidden", function () { + // Initially, only showTitle and hideTitle are available + expect(proxy.hideTitle).toBeUndefined(); + proxy.showTitle(); + + // Should have set titled state + expect(testElement.titled).toBeTruthy(); + + // Should also have changed methods available + expect(proxy.showTitle).toBeUndefined(); + proxy.hideTitle(); + + // Should have cleared titled state + expect(testElement.titled).toBeFalsy(); + + // Available methods should have changed again + expect(proxy.hideTitle).toBeUndefined(); + proxy.showTitle(); + }); + }); } ); diff --git a/platform/forms/test/controllers/ColorControllerSpec.js b/platform/forms/test/controllers/ColorControllerSpec.js index c8d432d91b..ce449bc98a 100644 --- a/platform/forms/test/controllers/ColorControllerSpec.js +++ b/platform/forms/test/controllers/ColorControllerSpec.js @@ -41,7 +41,7 @@ define( count += 1; set[color] = true; }); - }) + }); // Size of set should be number of colors if all were unique expect(Object.keys(set).length).toEqual(count); From 3d86871c1d881dc6a709b9803f80dc3cd85165c0 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Feb 2015 09:25:14 -0800 Subject: [PATCH 20/21] [Fixed Position] Cache name/value Cache name/value for telemetry elements to avoid flashing after drag gestures, WTD-881. --- platform/features/layout/src/FixedController.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index f795093dd4..afef3355b4 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -24,6 +24,8 @@ define( subscription, cellStyles = [], elementProxies = [], + names = {}, // Cache names by ID + values = {}, // Cache values by ID elementProxiesById = {}, selection; @@ -67,10 +69,12 @@ define( var id = telemetryObject && telemetryObject.getId(); if (id) { (elementProxiesById[id] || []).forEach(function (element) { - element.name = telemetryObject.getModel().name; - element.value = telemetryFormatter.formatRangeValue( + names[id] = telemetryObject.getModel().name; + values[id] = telemetryFormatter.formatRangeValue( subscription.getRangeValue(telemetryObject) ); + element.name = names[id]; + element.value = values[id]; }); } } @@ -127,6 +131,9 @@ define( elementProxies.forEach(function (elementProxy) { var id = elementProxy.id; if (elementProxy.element.type === 'fixed.telemetry') { + // Provide it a cached name/value to avoid flashing + elementProxy.name = names[id]; + elementProxy.value = values[id]; elementProxiesById[id] = elementProxiesById[id] || []; elementProxiesById[id].push(elementProxy); } From 1e76264d82ff3362c032d28f9e547b4c87671ee5 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Feb 2015 09:47:00 -0800 Subject: [PATCH 21/21] [Edit] Avoid extra commits Avoid issuing extra commit calls from the Edit toolbar; this can cause disruption of state from other views (such as Fixed Position), WTD-881. --- .../edit/src/representers/EditToolbar.js | 11 +++++++- .../representers/EditToolbarRepresenter.js | 16 ++++++++---- .../EditToolbarRepresenterSpec.js | 26 +++++++++++++++++++ .../edit/test/representers/EditToolbarSpec.js | 9 +++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/edit/src/representers/EditToolbar.js b/platform/commonUI/edit/src/representers/EditToolbar.js index 522a808aaf..8d5881a5a5 100644 --- a/platform/commonUI/edit/src/representers/EditToolbar.js +++ b/platform/commonUI/edit/src/representers/EditToolbar.js @@ -34,14 +34,20 @@ define( // Update value for this property in all elements of the // selection which have this property. function updateProperties(property, value) { + var changed = false; + // Update property in a selected element function updateProperty(selected) { // Ignore selected elements which don't have this property if (selected[property] !== undefined) { // Check if this is a setter, or just assignable if (typeof selected[property] === 'function') { + changed = + changed || (selected[property]() !== value); selected[property](value); } else { + changed = + changed || (selected[property] !== value); selected[property] = value; } } @@ -49,6 +55,9 @@ define( // Update property in all selected elements selection.forEach(updateProperty); + + // Return whether or not anything changed + return changed; } // Look up the current value associated with a property @@ -220,7 +229,7 @@ define( * @param value the new value to convey to the selection */ updateState: function (index, value) { - updateProperties(properties[index], value); + return updateProperties(properties[index], value); } }; } diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index b17c7f7d88..a1852e3bcd 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -51,11 +51,17 @@ define( // Update selection models to match changed toolbar state function updateState(state) { // Update underlying state based on toolbar changes - (state || []).forEach(function (value, index) { - toolbar.updateState(index, value); - }); - // Commit the changes. - commit("Changes from toolbar."); + var changed = (state || []).map(function (value, index) { + return toolbar.updateState(index, value); + }).reduce(function (a, b) { + return a || b; + }, false); + + // Only commit if something actually changed + if (changed) { + // Commit the changes. + commit("Changes from toolbar."); + } } // Initialize toolbar (expose object to parent scope) diff --git a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js index 56093b5e86..b3c5c64a1f 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js @@ -91,6 +91,32 @@ define( // Should have updated the original object expect(testObject.k).toEqual(456); + + // Should have committed the change + expect(mockScope.commit).toHaveBeenCalled(); + }); + + it("does not commit if nothing changed", function () { + var testObject = { k: 123 }; + + // Provide a view which has a toolbar + representer.represent({ + toolbar: { sections: [ { items: [ { property: 'k' } ] } ] } + }); + + // Update the selection + mockScope.selection.push(testObject); + expect(mockScope.$watchCollection.mostRecentCall.args[0]) + .toEqual('selection'); // Make sure we're using right watch + mockScope.$watchCollection.mostRecentCall.args[1]([testObject]); + + // Invoke the first watch (assumed to be for toolbar state) + mockScope.$watchCollection.calls[0].args[1]( + mockScope.$parent.testToolbar.state + ); + + // Should have committed the change + expect(mockScope.commit).not.toHaveBeenCalled(); }); }); diff --git a/platform/commonUI/edit/test/representers/EditToolbarSpec.js b/platform/commonUI/edit/test/representers/EditToolbarSpec.js index ea2fa928f6..452b59cefd 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarSpec.js @@ -143,6 +143,15 @@ define( expect(testABC.a).toHaveBeenCalledWith("new value"); }); + it("provides a return value describing update status", function () { + // Should return true if actually updated, otherwise false + var key; + toolbar.setSelection([ testABC ]); + key = toolbar.getStructure().sections[0].items[0].key; + expect(toolbar.updateState(key, testABC.a)).toBeFalsy(); + expect(toolbar.updateState(key, "new value")).toBeTruthy(); + }); + it("removes inapplicable items", function () { // First, verify with all items toolbar.setSelection([ testABC ]);