Merge remote-tracking branch 'origin/open929' into open-master

platform/commonUI/edit/src/representers/EditToolbarRepresenter.js

@@ -1,8 +1,8 @@
 /*global define*/
 
 define(
-    ['./EditToolbar'],
+    ['./EditToolbar', './EditToolbarSelection'],
-    function (EditToolbar) {
+    function (EditToolbar, EditToolbarSelection) {
         "use strict";
 
         // No operation
@@ -81,8 +81,8 @@ define(
                 var definition = (representation || {}).toolbar || {};
                 // Expose the toolbar object to the parent scope
                 initialize(definition);
-                // Clear any existing selection
+                // Create a selection scope
-                scope.selection = [];
+                scope.selection = new EditToolbarSelection();
                 // Initialize toolbar to an empty selection
                 updateSelection([]);
             }
@@ -101,7 +101,7 @@ define(
                 // Detect and handle changes to state from the toolbar
                 scope.$watchCollection(getState, updateState);
                 // Watch for changes in the current selection state
-                scope.$watchCollection("selection", updateSelection);
+                scope.$watchCollection("selection.all()", updateSelection);
                 // Expose toolbar state under that name
                 scope.$parent[attrs.toolbar] = toolbarObject;
             }

platform/commonUI/edit/src/representers/EditToolbarSelection.js

@@ -6,27 +6,23 @@ define(
         "use strict";
 
         /**
-         * Tracks selection state for Layout and Fixed Position views.
+         * Tracks selection state for editable views. Selection is
-         * This manages and mutates the provided selection array in-place,
+         * implemented such that (from the toolbar's perspective)
-         * and takes care to only modify the array elements it manages
+         * up to two objects can be "selected" at any given time:
-         * (the view's proxy, and the single selection); selections may be
-         * added or removed elsewhere provided that similar care is taken
-         * elsewhere.
          *
-         * @param {Array} selection the selection array from the view's scope
+         * * The view proxy (see the `proxy` method), which provides
-         * @param [proxy] an object which represents the selection of the view
+         *   an interface for interacting with the view itself (e.g.
-         *        itself (which handles view-level toolbar behavior)
+         *   for buttons like "Add")
+         * * The selection, for single selected elements within the
+         *   view.
+         *
+         * @constructor
          */
-        function LayoutSelection(selection, proxy) {
+        function EditToolbarSelection() {
-            var selecting = false,
+            var selection = [ {} ],
+                selecting = false,
                 selected;
 
-            // Find the proxy in the array; our selected objects will be
-            // positioned next to that
-            function proxyIndex() {
-                return selection.indexOf(proxy);
-            }
-
             // Remove the currently-selected object
             function deselect() {
                 // Nothing to do if we don't have a selected object
@@ -36,7 +32,7 @@ define(
                     selected = undefined;
 
                     // Remove the selection
-                    selection.splice(proxyIndex() + 1, 1);
+                    selection.pop();
 
                     return true;
                 }
@@ -45,11 +41,8 @@ define(
 
             // Select an object
             function select(obj) {
-                // We want this selection to end up near the proxy
-                var index = proxyIndex() + 1;
-
                 // Proxy is always selected
-                if (obj === proxy) {
+                if (obj === selection[0]) {
                     return false;
                 }
 
@@ -60,25 +53,14 @@ define(
                 selected = obj;
                 selecting = true;
 
-                // Are we at the end of the array?
+                // Add the selection
-                if (selection.length === index) {
+                selection.push(obj);
-                    // Add it to the end
-                    selection.push(obj);
-                } else {
-                    // Splice it into the array
-                    selection.splice(index, 0, obj);
-                }
             }
 
-            // Remove any selected object, and the proxy itself
-            function destroy() {
-                deselect();
-                selection.splice(proxyIndex(), 1);
-            }
 
             // Check if an object is selected
             function isSelected(obj) {
-                return (obj === selected) || (obj === proxy);
+                return (obj === selected) || (obj === selection[0]);
             }
 
             // Getter for current selection
@@ -86,8 +68,18 @@ define(
                 return selected;
             }
 
-            // Start with the proxy selected
+            // Getter/setter for view proxy
-            selection.push(proxy);
+            function proxy(p) {
+                if (arguments.length > 0) {
+                    selection[0] = p;
+                }
+                return selection[0];
+            }
+
+            // Getter for the full array of selected objects (incl. view proxy)
+            function all() {
+                return selection;
+            }
 
             return {
                 /**
@@ -112,15 +104,22 @@ define(
                  */
                 get: get,
                 /**
-                 * Clear the selection, including the proxy, and dispose
+                 * Get/set the view proxy (for toolbar actions taken upon
-                 * of this selection scope. No other calls to methods on
+                 * the view itself.)
-                 * this object are expected after `destroy` has been
+                 * @param [proxy] the view proxy (if setting)
-                 * called; their behavior will be undefined.
+                 * @returns the current view proxy
                  */
-                destroy: destroy
+                proxy: proxy,
+                /**
+                 * Get an array containing all selections, including the
+                 * selection proxy. It is generally not advisable to
+                 * mutate this array directly.
+                 * @returns {Array} all selections
+                 */
+                all: all
             };
         }
 
-        return LayoutSelection;
+        return EditToolbarSelection;
     }
 );

platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js

@@ -77,9 +77,9 @@ define(
                 });
 
                 // Update the selection
-                mockScope.selection.push(testObject);
+                mockScope.selection.select(testObject);
                 expect(mockScope.$watchCollection.mostRecentCall.args[0])
-                    .toEqual('selection'); // Make sure we're using right watch
+                    .toEqual('selection.all()'); // Make sure we're using right watch
                 mockScope.$watchCollection.mostRecentCall.args[1]([testObject]);
 
                 // Update the state
@@ -105,9 +105,9 @@ define(
                 });
 
                 // Update the selection
-                mockScope.selection.push(testObject);
+                mockScope.selection.select(testObject);
                 expect(mockScope.$watchCollection.mostRecentCall.args[0])
-                    .toEqual('selection'); // Make sure we're using right watch
+                    .toEqual('selection.all()'); // Make sure we're using right watch
                 mockScope.$watchCollection.mostRecentCall.args[1]([testObject]);
 
                 // Invoke the first watch (assumed to be for toolbar state)

platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js

@@ -1,44 +1,47 @@
 /*global define,describe,it,expect,beforeEach,jasmine,xit*/
 
 define(
-    ['../src/LayoutSelection'],
+    ['../../src/representers/EditToolbarSelection'],
-    function (LayoutSelection) {
+    function (EditToolbarSelection) {
         "use strict";
 
-        describe("Layout/fixed position selection manager", function () {
+        describe("The Edit mode selection manager", function () {
-            var testSelection,
+            var testProxy,
-                testProxy,
                 testElement,
                 otherElement,
                 selection;
 
             beforeEach(function () {
-                testSelection = [];
                 testProxy = { someKey: "some value" };
                 testElement = { someOtherKey: "some other value" };
                 otherElement = { yetAnotherKey: 42 };
-                selection = new LayoutSelection(testSelection, testProxy);
+                selection = new EditToolbarSelection();
+                selection.proxy(testProxy);
             });
 
             it("adds the proxy to the selection array", function () {
-                expect(testSelection).toEqual([testProxy]);
+                expect(selection.all()).toEqual([testProxy]);
+            });
+
+            it("exposes view proxy", function () {
+                expect(selection.proxy()).toBe(testProxy);
             });
 
             it("includes selected objects alongside the proxy", function () {
                 selection.select(testElement);
-                expect(testSelection).toEqual([testProxy, testElement]);
+                expect(selection.all()).toEqual([testProxy, testElement]);
             });
 
             it("allows elements to be deselected", function () {
                 selection.select(testElement);
                 selection.deselect();
-                expect(testSelection).toEqual([testProxy]);
+                expect(selection.all()).toEqual([testProxy]);
             });
 
             it("replaces old selections with new ones", function () {
                 selection.select(testElement);
                 selection.select(otherElement);
-                expect(testSelection).toEqual([testProxy, otherElement]);
+                expect(selection.all()).toEqual([testProxy, otherElement]);
             });
 
             it("allows retrieval of the current selection", function () {
@@ -57,17 +60,6 @@ define(
                 expect(selection.selected(otherElement)).toBeTruthy();
             });
 
-            it("cleans up the selection on destroy", function () {
-                selection.destroy();
-                expect(testSelection).toEqual([]);
-            });
-
-            it("preserves other elements in the array", function () {
-                testSelection.push(42);
-                selection.select(testElement);
-                expect(testSelection).toEqual([testProxy, testElement, 42]);
-            });
-
             it("considers the proxy to be selected", function () {
                 expect(selection.selected(testProxy)).toBeTruthy();
                 selection.select(testElement);
@@ -77,7 +69,7 @@ define(
 
             it("treats selection of the proxy as a no-op", function () {
                 selection.select(testProxy);
-                expect(testSelection).toEqual([testProxy]);
+                expect(selection.all()).toEqual([testProxy]);
             });
 
         });

platform/commonUI/edit/test/suite.json

@@ -18,5 +18,6 @@
     "objects/EditableModelCache",
     "representers/EditRepresenter",
     "representers/EditToolbar",
-    "representers/EditToolbarRepresenter"
+    "representers/EditToolbarRepresenter",
+    "representers/EditToolbarSelection"
 ]

platform/features/layout/src/FixedController.js

@@ -1,11 +1,12 @@
 /*global define*/
 
 define(
-    ['./LayoutSelection', './FixedProxy', './elements/ElementProxies', './FixedDragHandle'],
+    ['./FixedProxy', './elements/ElementProxies', './FixedDragHandle'],
-    function (LayoutSelection, FixedProxy, ElementProxies, FixedDragHandle) {
+    function (FixedProxy, ElementProxies, FixedDragHandle) {
         "use strict";
 
-        var DEFAULT_GRID_SIZE = [64, 16],
+        var DEFAULT_DIMENSIONS = [ 2, 1 ],
+            DEFAULT_GRID_SIZE = [64, 16],
             DEFAULT_GRID_EXTENT = [4, 4];
 
         /**
@@ -28,7 +29,6 @@ define(
                 elementProxiesById = {},
                 handles = [],
                 moveHandle,
-                viewProxy,
                 selection;
 
             // Refresh cell styles (e.g. because grid extent changed)
@@ -198,6 +198,14 @@ define(
                     telemetrySubscriber.subscribe(domainObject, updateValues);
             }
 
+            // Handle changes in the object's composition
+            function updateComposition(ids) {
+                // Populate panel positions
+                // TODO: Ensure defaults here
+                // Resubscribe - objects in view have changed
+                subscribe($scope.domainObject);
+            }
+
             // Add an element to this view
             function addElement(element) {
                 // Ensure that configuration field is populated
@@ -218,93 +226,28 @@ define(
                 }
             }
 
-            // Add a telemetry element to this view
-            function addTelemetryElement(id, x, y) {
-                viewProxy.add("fixed.telemetry", { id: id, x: x, y: y });
-            }
-
-            // Ensure that all telemetry elements have elements in view
-            function ensureElements(ids) {
-                var contained = {},
-                    found = {};
-
-                // Track that a telemetry element is in the view
-                function track(element) {
-                    if (element.type === 'fixed.telemetry') {
-                        found[element.id] = true;
-                    }
-                }
-
-                // Used to filter down to elements not yet present
-                function notFound(id) {
-                    return !found[id];
-                }
-
-                // Add a telemetry element
-                function add(id, index) {
-                    addTelemetryElement(id, 0, index);
-                }
-
-                // Build list of all found elements
-                (($scope.configuration || {}).elements || []).forEach(track);
-
-                // Add in telemetry elements where needed
-                (ids || []).filter(notFound).forEach(add);
-            }
-
-            // Remove telemetry elements which don't match set of contained ids
-            function removeOtherElements(ids) {
-                // Set of ids, to simplify lookup
-                var contained = {},
-                    elements = ($scope.configuration || {}).elements;
-
-                // Track that an id is present; used to build set
-                function track(id) {
-                    contained[id] = true;
-                }
-
-                // Check if an element is still valid
-                function valid(element) {
-                    return (element.type !== "fixed.telemetry") ||
-                            contained[element.id];
-                }
-
-                // Only need to remove elements if any have been defined
-                if (Array.isArray(elements)) {
-                    // Build set of contained ids
-                    ids.forEach(track);
-                    // Filter out removed telemetry elements
-                    $scope.configuration.elements = elements.filter(valid);
-                    // Refresh elements exposed to template
-                    refreshElements();
-                }
-            }
-
-            // Handle changes in the object's composition
-            function updateComposition(ids) {
-                // Remove any obsolete telemetry elements
-                removeOtherElements(ids);
-                // Populate panel positions
-                ensureElements(ids);
-                // Resubscribe - objects in view have changed
-                subscribe($scope.domainObject);
-            }
-
             // Position a panel after a drop event
             function handleDrop(e, id, position) {
                 // Store the position of this element.
-                addTelemetryElement(
+                addElement({
-                    id,
+                    type: "fixed.telemetry",
-                    Math.floor(position.x / gridSize[0]),
+                    x: Math.floor(position.x / gridSize[0]),
-                    Math.floor(position.y / gridSize[1])
+                    y: Math.floor(position.y / gridSize[1]),
-                );
+                    id: id,
+                    stroke: "transparent",
+                    color: "#cccccc",
+                    titled: true,
+                    width: DEFAULT_DIMENSIONS[0],
+                    height: DEFAULT_DIMENSIONS[1]
+                });
             }
 
+            // Track current selection state
+            selection = $scope.selection;
 
-            //  Track current selection state
+            // Expose the view's selection proxy
-            viewProxy = new FixedProxy(addElement, $q, dialogService);
+            if (selection) {
-            if (Array.isArray($scope.selection)) {
+                selection.proxy(new FixedProxy(addElement, $q, dialogService));
-                selection = new LayoutSelection($scope.selection, viewProxy);
             }
 
             // Refresh list of elements whenever model changes

platform/features/layout/src/FixedProxy.js

@@ -21,14 +21,9 @@ define(
                 /**
                  * Add a new visual element to this view.
                  */
-                add: function (type, state) {
+                add: function (type) {
                     // Place a configured element into the view configuration
                     function addElement(element) {
-                        // Populate element with additional state
-                        Object.keys(state || {}).forEach(function (k) {
-                            element[k] = state[k];
-                        });
-
                         // Configure common properties of the element
                         element.x = element.x || 0;
                         element.y = element.y || 0;

platform/features/layout/src/elements/ElementFactory.js

@@ -25,13 +25,6 @@ define(
                     fill: "transparent",
                     stroke: "transparent",
                     color: "#cccccc"
-                },
-                "fixed.telemetry": {
-                    stroke: "transparent",
-                    color: "#cccccc",
-                    titled: true,
-                    width: 2,
-                    height: 1
                 }
             },
             DIALOGS = {

platform/features/layout/test/FixedControllerSpec.js

@@ -13,7 +13,6 @@ define(
                 mockFormatter,
                 mockDomainObject,
                 mockSubscription,
-                mockPromise,
                 testGrid,
                 testModel,
                 testValues,
@@ -78,10 +77,6 @@ define(
                     'subscription',
                     [ 'unsubscribe', 'getTelemetryObjects', 'getRangeValue' ]
                 );
-                mockPromise = jasmine.createSpyObj(
-                    'promise',
-                    [ 'then' ]
-                );
 
                 testGrid = [ 123, 456 ];
                 testModel = {
@@ -107,9 +102,10 @@ define(
                 });
                 mockScope.model = testModel;
                 mockScope.configuration = testConfiguration;
-                mockScope.selection = []; // Act like edit mode
+                mockScope.selection = jasmine.createSpyObj(
-                mockQ.when.andReturn(mockPromise);
+                    'selection',
-                mockPromise.then.andCallFake(function (cb) { cb({}); });
+                    [ 'select', 'get', 'selected', 'deselect', 'proxy' ]
+                );
 
                 controller = new FixedController(
                     mockScope,
@@ -170,8 +166,8 @@ define(
 
                 elements = controller.getElements();
                 controller.select(elements[1]);
-                expect(controller.selected(elements[0])).toBeFalsy();
+                expect(mockScope.selection.select)
-                expect(controller.selected(elements[1])).toBeTruthy();
+                    .toHaveBeenCalledWith(elements[1]);
             });
 
             it("allows selection retrieval", function () {
@@ -184,6 +180,7 @@ define(
 
                 elements = controller.getElements();
                 controller.select(elements[1]);
+                mockScope.selection.get.andReturn(elements[1]);
                 expect(controller.selected()).toEqual(elements[1]);
             });
 
@@ -210,6 +207,12 @@ define(
                 elements = controller.getElements();
                 controller.select(elements[1]);
 
+                // Verify precondition
+                expect(mockScope.selection.select.calls.length).toEqual(1);
+
+                // Mimic selection behavior
+                mockScope.selection.get.andReturn(elements[1]);
+
                 elements[2].remove();
                 testModel.modified = 2;
                 findWatch("model.modified")(testModel.modified);
@@ -218,7 +221,7 @@ define(
                 // Verify removal, as test assumes this
                 expect(elements.length).toEqual(2);
 
-                expect(controller.selected(elements[1])).toBeTruthy();
+                expect(mockScope.selection.select.calls.length).toEqual(2);
             });
 
             it("provides values for telemetry elements", function () {
@@ -309,6 +312,12 @@ define(
                 expect(controller.getGridSize()).toEqual(testGrid);
             });
 
+            it("exposes a view-level selection proxy", function () {
+                expect(mockScope.selection.proxy).toHaveBeenCalledWith(
+                    jasmine.any(Object)
+                );
+            });
+
             it("exposes drag handles", function () {
                 var handles;
 
@@ -354,6 +363,7 @@ define(
                 testModel.modified = 1;
                 findWatch("model.modified")(testModel.modified);
                 controller.select(controller.getElements()[1]);
+                mockScope.selection.get.andReturn(controller.getElements()[1]);
 
                 // Get style
                 oldStyle = controller.selected().style;
@@ -370,19 +380,6 @@ define(
                 // Style should have been updated
                 expect(controller.selected().style).not.toEqual(oldStyle);
             });
-
-            it("ensures elements in view match elements in composition", function () {
-                // View should ensure that at least one element is present
-                // for each id, and then unused ids do not have elements.
-                mockScope.model = testModel;
-                testModel.composition = [ 'b', 'd' ];
-                findWatch("model.composition")(mockScope.model.composition);
-
-                // Should have a new element for d; should not have elements for a, c
-                expect(testConfiguration.elements.length).toEqual(2);
-                expect(testConfiguration.elements[0].id).toEqual('b');
-                expect(testConfiguration.elements[1].id).toEqual('d');
-            });
         });
     }
 );

platform/features/layout/test/suite.json

@@ -4,7 +4,6 @@
     "FixedProxy",
     "LayoutController",
     "LayoutDrag",
-    "LayoutSelection",
     "elements/AccessorMutator",
     "elements/BoxProxy",
     "elements/ElementFactory",