diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index a1852e3bcd..d30445061f 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -1,8 +1,8 @@ /*global define*/ define( - ['./EditToolbar'], - function (EditToolbar) { + ['./EditToolbar', './EditToolbarSelection'], + 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 - scope.selection = []; + // Create a selection scope + 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; } diff --git a/platform/features/layout/src/LayoutSelection.js b/platform/commonUI/edit/src/representers/EditToolbarSelection.js similarity index 50% rename from platform/features/layout/src/LayoutSelection.js rename to platform/commonUI/edit/src/representers/EditToolbarSelection.js index bb1574f0ff..bc6e2fa861 100644 --- a/platform/features/layout/src/LayoutSelection.js +++ b/platform/commonUI/edit/src/representers/EditToolbarSelection.js @@ -6,27 +6,23 @@ define( "use strict"; /** - * Tracks selection state for Layout and Fixed Position views. - * This manages and mutates the provided selection array in-place, - * and takes care to only modify the array elements it manages - * (the view's proxy, and the single selection); selections may be - * added or removed elsewhere provided that similar care is taken - * elsewhere. + * Tracks selection state for editable views. Selection is + * implemented such that (from the toolbar's perspective) + * up to two objects can be "selected" at any given time: * - * @param {Array} selection the selection array from the view's scope - * @param [proxy] an object which represents the selection of the view - * itself (which handles view-level toolbar behavior) + * * The view proxy (see the `proxy` method), which provides + * an interface for interacting with the view itself (e.g. + * for buttons like "Add") + * * The selection, for single selected elements within the + * view. + * + * @constructor */ - function LayoutSelection(selection, proxy) { - var selecting = false, + function EditToolbarSelection() { + 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? - if (selection.length === index) { - // Add it to the end - selection.push(obj); - } else { - // Splice it into the array - selection.splice(index, 0, obj); - } + // Add the selection + selection.push(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 - selection.push(proxy); + // Getter/setter for view 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 - * of this selection scope. No other calls to methods on - * this object are expected after `destroy` has been - * called; their behavior will be undefined. + * Get/set the view proxy (for toolbar actions taken upon + * the view itself.) + * @param [proxy] the view proxy (if setting) + * @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; } ); \ No newline at end of file diff --git a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js index b3c5c64a1f..0d788c834f 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js +++ b/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) diff --git a/platform/features/layout/test/LayoutSelectionSpec.js b/platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js similarity index 70% rename from platform/features/layout/test/LayoutSelectionSpec.js rename to platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js index 3712afec3f..7b5c2b775a 100644 --- a/platform/features/layout/test/LayoutSelectionSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js @@ -1,44 +1,47 @@ /*global define,describe,it,expect,beforeEach,jasmine,xit*/ define( - ['../src/LayoutSelection'], - function (LayoutSelection) { + ['../../src/representers/EditToolbarSelection'], + function (EditToolbarSelection) { "use strict"; - describe("Layout/fixed position selection manager", function () { - var testSelection, - testProxy, + describe("The Edit mode selection manager", function () { + var 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]); }); }); diff --git a/platform/commonUI/edit/test/suite.json b/platform/commonUI/edit/test/suite.json index f3ace073eb..46f0443b41 100644 --- a/platform/commonUI/edit/test/suite.json +++ b/platform/commonUI/edit/test/suite.json @@ -18,5 +18,6 @@ "objects/EditableModelCache", "representers/EditRepresenter", "representers/EditToolbar", - "representers/EditToolbarRepresenter" + "representers/EditToolbarRepresenter", + "representers/EditToolbarSelection" ] \ No newline at end of file diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index f2f8a354ee..850441d15f 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -1,11 +1,12 @@ /*global define*/ define( - ['./LayoutSelection', './FixedProxy', './elements/ElementProxies', './FixedDragHandle'], - function (LayoutSelection, FixedProxy, ElementProxies, FixedDragHandle) { + ['./FixedProxy', './elements/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( - id, - Math.floor(position.x / gridSize[0]), - Math.floor(position.y / gridSize[1]) - ); + addElement({ + type: "fixed.telemetry", + x: Math.floor(position.x / gridSize[0]), + 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 - viewProxy = new FixedProxy(addElement, $q, dialogService); - if (Array.isArray($scope.selection)) { - selection = new LayoutSelection($scope.selection, viewProxy); + // Expose the view's selection proxy + if (selection) { + selection.proxy(new FixedProxy(addElement, $q, dialogService)); } // Refresh list of elements whenever model changes diff --git a/platform/features/layout/src/FixedProxy.js b/platform/features/layout/src/FixedProxy.js index 846f16aa84..4b5b4312e0 100644 --- a/platform/features/layout/src/FixedProxy.js +++ b/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; diff --git a/platform/features/layout/src/elements/ElementFactory.js b/platform/features/layout/src/elements/ElementFactory.js index 3883ba66a9..d80a8c420c 100644 --- a/platform/features/layout/src/elements/ElementFactory.js +++ b/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 = { diff --git a/platform/features/layout/test/FixedControllerSpec.js b/platform/features/layout/test/FixedControllerSpec.js index 4d976acd7f..d94c93fc13 100644 --- a/platform/features/layout/test/FixedControllerSpec.js +++ b/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 - mockQ.when.andReturn(mockPromise); - mockPromise.then.andCallFake(function (cb) { cb({}); }); + mockScope.selection = jasmine.createSpyObj( + 'selection', + [ '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(controller.selected(elements[1])).toBeTruthy(); + expect(mockScope.selection.select) + .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'); - }); }); } ); \ No newline at end of file diff --git a/platform/features/layout/test/suite.json b/platform/features/layout/test/suite.json index 684c8fda12..3bc1a259c1 100644 --- a/platform/features/layout/test/suite.json +++ b/platform/features/layout/test/suite.json @@ -4,7 +4,6 @@ "FixedProxy", "LayoutController", "LayoutDrag", - "LayoutSelection", "elements/AccessorMutator", "elements/BoxProxy", "elements/ElementFactory",