From 6c97815efb8594df62c1bbc6ddf94cc36f03fd4b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:31:34 -0800 Subject: [PATCH 1/7] [Edit] Add Selection class Add class to handle selection, which will be synchronized to the toolbar. WTD-929. --- .../src/representers/EditToolbarSelection.js | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 platform/commonUI/edit/src/representers/EditToolbarSelection.js diff --git a/platform/commonUI/edit/src/representers/EditToolbarSelection.js b/platform/commonUI/edit/src/representers/EditToolbarSelection.js new file mode 100644 index 0000000000..8bec96d486 --- /dev/null +++ b/platform/commonUI/edit/src/representers/EditToolbarSelection.js @@ -0,0 +1,124 @@ +/*global define*/ + +define( + [], + function () { + "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. + * + * @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) + */ + function EditToolbarSelection() { + var selection = [ {} ], + selecting = false, + selected; + + // Remove the currently-selected object + function deselect() { + // Nothing to do if we don't have a selected object + if (selecting) { + // Clear state tracking + selecting = false; + selected = undefined; + + // Remove the selection + selection.pop(); + + return true; + } + return false; + } + + // Select an object + function select(obj) { + // Proxy is always selected + if (obj === selection[0]) { + return false; + } + + // Clear any existing selection + deselect(); + + // Note the current selection state + selected = obj; + selecting = true; + + // Add the selection + selection.push(obj); + } + + + // Check if an object is selected + function isSelected(obj) { + return (obj === selected) || (obj === selection[0]); + } + + // Getter for current selection + function get() { + return selected; + } + + // 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 { + /** + * Check if an object is currently selected. + * @returns true if selected, otherwise false + */ + selected: isSelected, + /** + * Select an object. + * @param obj the object to select + * @returns {boolean} true if selection changed + */ + select: select, + /** + * Clear the current selection. + * @returns {boolean} true if selection changed + */ + deselect: deselect, + /** + * Get the currently-selected object. + * @returns the currently selected object + */ + get: get, + /** + * 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 + */ + 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 EditToolbarSelection; + } +); \ No newline at end of file From bc69f19012c4b44b3e74969354c6d1cbca9d29f1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:34:03 -0800 Subject: [PATCH 2/7] [Edit] Utilize EditToolbarSelection Utilize newer, more general-purpose selector from the representer responsible for binding selection scope to toolbar. WTD-929. --- .../edit/src/representers/EditToolbarRepresenter.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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; } From ea37c636ee94992a1ec6cf0771e1d49ecb964b24 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:35:28 -0800 Subject: [PATCH 3/7] [Fixed Position] Use updated selection mechanism Use updated, more general approach to handling selections. WTD-929. --- platform/features/layout/src/FixedController.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index afef3355b4..fc71829e8b 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -1,8 +1,8 @@ /*global define*/ define( - ['./LayoutDrag', './LayoutSelection', './FixedProxy', './elements/ElementProxies'], - function (LayoutDrag, LayoutSelection, FixedProxy, ElementProxies) { + ['./LayoutDrag', './FixedProxy', './elements/ElementProxies'], + function (LayoutDrag, FixedProxy, ElementProxies) { "use strict"; var DEFAULT_DIMENSIONS = [ 2, 1 ], @@ -208,14 +208,8 @@ define( }); } - // Track current selection state - if (Array.isArray($scope.selection)) { - selection = new LayoutSelection( - $scope.selection, - new FixedProxy(addElement, $q, dialogService) - ); - } + selection = $scope.selection; // Refresh list of elements whenever model changes $scope.$watch("model.modified", refreshElements); From d7e962e4b1f685cb0b15814a035b4ef5f505f515 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:43:26 -0800 Subject: [PATCH 4/7] [Edit] Update tests Update tests for Edit mode bundle to include the general-purpose selection API, WTD-929. --- .../src/representers/EditToolbarSelection.js | 19 ++--- .../EditToolbarRepresenterSpec.js | 8 +- .../representers/EditToolbarSelectionSpec.js | 77 +++++++++++++++++++ platform/commonUI/edit/test/suite.json | 3 +- 4 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js diff --git a/platform/commonUI/edit/src/representers/EditToolbarSelection.js b/platform/commonUI/edit/src/representers/EditToolbarSelection.js index 8bec96d486..bc6e2fa861 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarSelection.js +++ b/platform/commonUI/edit/src/representers/EditToolbarSelection.js @@ -6,16 +6,17 @@ 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 EditToolbarSelection() { var selection = [ {} ], 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/commonUI/edit/test/representers/EditToolbarSelectionSpec.js b/platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js new file mode 100644 index 0000000000..7b5c2b775a --- /dev/null +++ b/platform/commonUI/edit/test/representers/EditToolbarSelectionSpec.js @@ -0,0 +1,77 @@ +/*global define,describe,it,expect,beforeEach,jasmine,xit*/ + +define( + ['../../src/representers/EditToolbarSelection'], + function (EditToolbarSelection) { + "use strict"; + + describe("The Edit mode selection manager", function () { + var testProxy, + testElement, + otherElement, + selection; + + beforeEach(function () { + testProxy = { someKey: "some value" }; + testElement = { someOtherKey: "some other value" }; + otherElement = { yetAnotherKey: 42 }; + selection = new EditToolbarSelection(); + selection.proxy(testProxy); + }); + + it("adds the proxy to the selection array", function () { + 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(selection.all()).toEqual([testProxy, testElement]); + }); + + it("allows elements to be deselected", function () { + selection.select(testElement); + selection.deselect(); + expect(selection.all()).toEqual([testProxy]); + }); + + it("replaces old selections with new ones", function () { + selection.select(testElement); + selection.select(otherElement); + expect(selection.all()).toEqual([testProxy, otherElement]); + }); + + it("allows retrieval of the current selection", function () { + selection.select(testElement); + expect(selection.get()).toBe(testElement); + selection.select(otherElement); + expect(selection.get()).toBe(otherElement); + }); + + it("can check if an element is selected", function () { + selection.select(testElement); + expect(selection.selected(testElement)).toBeTruthy(); + expect(selection.selected(otherElement)).toBeFalsy(); + selection.select(otherElement); + expect(selection.selected(testElement)).toBeFalsy(); + expect(selection.selected(otherElement)).toBeTruthy(); + }); + + it("considers the proxy to be selected", function () { + expect(selection.selected(testProxy)).toBeTruthy(); + selection.select(testElement); + // Even when something else is selected... + expect(selection.selected(testProxy)).toBeTruthy(); + }); + + it("treats selection of the proxy as a no-op", function () { + selection.select(testProxy); + expect(selection.all()).toEqual([testProxy]); + }); + + }); + } +); diff --git a/platform/commonUI/edit/test/suite.json b/platform/commonUI/edit/test/suite.json index 5744ff8ea8..7c4d892f81 100644 --- a/platform/commonUI/edit/test/suite.json +++ b/platform/commonUI/edit/test/suite.json @@ -17,5 +17,6 @@ "objects/EditableDomainObjectCache", "objects/EditableModelCache", "representers/EditToolbar", - "representers/EditToolbarRepresenter" + "representers/EditToolbarRepresenter", + "representers/EditToolbarSelection" ] \ No newline at end of file From 948b661fe7bbd266f6ed63e1c625459b2469c7c3 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:47:44 -0800 Subject: [PATCH 5/7] [Fixed Position] Update specs Update specs to reflect usage of generalized selection mechanism, WTD-929. --- .../features/layout/test/FixedControllerSpec.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/platform/features/layout/test/FixedControllerSpec.js b/platform/features/layout/test/FixedControllerSpec.js index c8b943cc62..5028812c0e 100644 --- a/platform/features/layout/test/FixedControllerSpec.js +++ b/platform/features/layout/test/FixedControllerSpec.js @@ -102,7 +102,10 @@ define( }); mockScope.model = testModel; mockScope.configuration = testConfiguration; - mockScope.selection = []; // Act like edit mode + mockScope.selection = jasmine.createSpyObj( + 'selection', + [ 'select', 'get', 'selected', 'deselect' ] + ); controller = new FixedController( mockScope, @@ -163,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 selections to be cleared", function () { @@ -190,6 +193,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); @@ -198,7 +207,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 () { From 6364296967418897cb9aa5e94e768660be7c5dfb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:48:53 -0800 Subject: [PATCH 6/7] [Fixed Position] Remove obsolete script Remove obsolete script for selection management in Fixed Position view; this has been generalized to become part of Edit mode. WTD-929. --- .../features/layout/src/LayoutSelection.js | 126 ------------------ .../layout/test/LayoutSelectionSpec.js | 85 ------------ platform/features/layout/test/suite.json | 1 - 3 files changed, 212 deletions(-) delete mode 100644 platform/features/layout/src/LayoutSelection.js delete mode 100644 platform/features/layout/test/LayoutSelectionSpec.js diff --git a/platform/features/layout/src/LayoutSelection.js b/platform/features/layout/src/LayoutSelection.js deleted file mode 100644 index bb1574f0ff..0000000000 --- a/platform/features/layout/src/LayoutSelection.js +++ /dev/null @@ -1,126 +0,0 @@ -/*global define*/ - -define( - [], - function () { - "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. - * - * @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) - */ - function LayoutSelection(selection, proxy) { - var 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 - if (selecting) { - // Clear state tracking - selecting = false; - selected = undefined; - - // Remove the selection - selection.splice(proxyIndex() + 1, 1); - - return true; - } - return false; - } - - // 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) { - return false; - } - - // Clear any existing selection - deselect(); - - // Note the current selection state - 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); - } - } - - // 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); - } - - // Getter for current selection - function get() { - return selected; - } - - // Start with the proxy selected - selection.push(proxy); - - return { - /** - * Check if an object is currently selected. - * @returns true if selected, otherwise false - */ - selected: isSelected, - /** - * Select an object. - * @param obj the object to select - * @returns {boolean} true if selection changed - */ - select: select, - /** - * Clear the current selection. - * @returns {boolean} true if selection changed - */ - deselect: deselect, - /** - * Get the currently-selected object. - * @returns the currently selected object - */ - 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. - */ - destroy: destroy - }; - } - - return LayoutSelection; - } -); \ No newline at end of file diff --git a/platform/features/layout/test/LayoutSelectionSpec.js b/platform/features/layout/test/LayoutSelectionSpec.js deleted file mode 100644 index 3712afec3f..0000000000 --- a/platform/features/layout/test/LayoutSelectionSpec.js +++ /dev/null @@ -1,85 +0,0 @@ -/*global define,describe,it,expect,beforeEach,jasmine,xit*/ - -define( - ['../src/LayoutSelection'], - function (LayoutSelection) { - "use strict"; - - describe("Layout/fixed position selection manager", function () { - var testSelection, - testProxy, - testElement, - otherElement, - selection; - - beforeEach(function () { - testSelection = []; - testProxy = { someKey: "some value" }; - testElement = { someOtherKey: "some other value" }; - otherElement = { yetAnotherKey: 42 }; - selection = new LayoutSelection(testSelection, testProxy); - }); - - it("adds the proxy to the selection array", function () { - expect(testSelection).toEqual([testProxy]); - }); - - it("includes selected objects alongside the proxy", function () { - selection.select(testElement); - expect(testSelection).toEqual([testProxy, testElement]); - }); - - it("allows elements to be deselected", function () { - selection.select(testElement); - selection.deselect(); - expect(testSelection).toEqual([testProxy]); - }); - - it("replaces old selections with new ones", function () { - selection.select(testElement); - selection.select(otherElement); - expect(testSelection).toEqual([testProxy, otherElement]); - }); - - it("allows retrieval of the current selection", function () { - selection.select(testElement); - expect(selection.get()).toBe(testElement); - selection.select(otherElement); - expect(selection.get()).toBe(otherElement); - }); - - it("can check if an element is selected", function () { - selection.select(testElement); - expect(selection.selected(testElement)).toBeTruthy(); - expect(selection.selected(otherElement)).toBeFalsy(); - selection.select(otherElement); - expect(selection.selected(testElement)).toBeFalsy(); - 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); - // Even when something else is selected... - expect(selection.selected(testProxy)).toBeTruthy(); - }); - - it("treats selection of the proxy as a no-op", function () { - selection.select(testProxy); - expect(testSelection).toEqual([testProxy]); - }); - - }); - } -); diff --git a/platform/features/layout/test/suite.json b/platform/features/layout/test/suite.json index 8f0eec06c3..774cc00819 100644 --- a/platform/features/layout/test/suite.json +++ b/platform/features/layout/test/suite.json @@ -3,7 +3,6 @@ "FixedProxy", "LayoutController", "LayoutDrag", - "LayoutSelection", "elements/AccessorMutator", "elements/BoxProxy", "elements/ElementFactory", From 3ce1064c4e0fff7bd4a3c80b911f87062f0949c4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 18:52:13 -0800 Subject: [PATCH 7/7] [Fixed Position] Expose view proxy Expose view proxy as selection state from the Fixed Position controller, to make Add button available under generalized selection mechanism, WTD-929. --- platform/features/layout/src/FixedController.js | 7 ++++++- platform/features/layout/test/FixedControllerSpec.js | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index fc71829e8b..60bc813c80 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -208,9 +208,14 @@ define( }); } - // Track current selection state + // Track current selection state selection = $scope.selection; + // Expose the view's selection proxy + if (selection) { + selection.proxy(new FixedProxy(addElement, $q, dialogService)); + } + // Refresh list of elements whenever model changes $scope.$watch("model.modified", refreshElements); diff --git a/platform/features/layout/test/FixedControllerSpec.js b/platform/features/layout/test/FixedControllerSpec.js index 5028812c0e..68562a0e28 100644 --- a/platform/features/layout/test/FixedControllerSpec.js +++ b/platform/features/layout/test/FixedControllerSpec.js @@ -104,7 +104,7 @@ define( mockScope.configuration = testConfiguration; mockScope.selection = jasmine.createSpyObj( 'selection', - [ 'select', 'get', 'selected', 'deselect' ] + [ 'select', 'get', 'selected', 'deselect', 'proxy' ] ); controller = new FixedController( @@ -297,6 +297,12 @@ define( // elements to size SVGs appropriately expect(controller.getGridSize()).toEqual(testGrid); }); + + it("exposes a view-level selection proxy", function () { + expect(mockScope.selection.proxy).toHaveBeenCalledWith( + jasmine.any(Object) + ); + }); }); } ); \ No newline at end of file