From 14894cf197313f350964f183aaf79e68d2027023 Mon Sep 17 00:00:00 2001 From: Pegah Sarram Date: Thu, 7 Dec 2017 14:59:12 -0800 Subject: [PATCH] [Fixed Position] Modify fixed position to use the Selection API Change method name to shouldSelect() as requested by the reviewer. Fix tests. Fixes #1848 --- .../src/controllers/ElementsController.js | 3 +- .../src/representers/EditToolbarSelection.js | 10 +- .../regions/src/InspectorController.js | 6 +- platform/features/layout/bundle.js | 3 +- .../features/layout/res/templates/fixed.html | 25 +-- .../features/layout/src/FixedController.js | 189 ++++++++++++------ .../features/layout/src/FixedDragHandle.js | 2 +- .../layout/src/elements/ElementProxy.js | 2 +- .../layout/test/FixedControllerSpec.js | 145 +++++++++----- src/selection/Selection.js | 2 +- 10 files changed, 248 insertions(+), 139 deletions(-) diff --git a/platform/commonUI/edit/src/controllers/ElementsController.js b/platform/commonUI/edit/src/controllers/ElementsController.js index 7f77eab56a..266a2b3a64 100644 --- a/platform/commonUI/edit/src/controllers/ElementsController.js +++ b/platform/commonUI/edit/src/controllers/ElementsController.js @@ -78,7 +78,8 @@ define( return; } - var selectedObjectComposition = selection[0].context.oldItem.useCapability('composition'); + var selected = selection[0].context.oldItem; + var selectedObjectComposition = selected && selected.useCapability('composition'); if (selectedObjectComposition) { selectedObjectComposition.then(function (composition) { diff --git a/platform/commonUI/edit/src/representers/EditToolbarSelection.js b/platform/commonUI/edit/src/representers/EditToolbarSelection.js index f0690dce7b..282640d1da 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarSelection.js +++ b/platform/commonUI/edit/src/representers/EditToolbarSelection.js @@ -44,11 +44,17 @@ define( this.selectedObj = undefined; openmct.selection.on('change', function (selection) { - if (selection[0] && selection[0].context.toolbar) { - this.select(selection[0].context.toolbar); + var selected = selection[0]; + + if (selected && selected.context.toolbar) { + this.select(selected.context.toolbar); } else { this.deselect(); } + + if (selected && selected.context.viewProxy) { + this.proxy(selected.context.viewProxy); + } }.bind(this)); } diff --git a/platform/commonUI/regions/src/InspectorController.js b/platform/commonUI/regions/src/InspectorController.js index 494ff7e368..0a18495d4c 100644 --- a/platform/commonUI/regions/src/InspectorController.js +++ b/platform/commonUI/regions/src/InspectorController.js @@ -50,7 +50,11 @@ define( view.show(container); } else { self.providerView = false; - $scope.inspectorKey = selection[0].context.oldItem.getCapability("type").typeDef.inspector; + var selectedItem = selection[0].context.oldItem; + + if (selectedItem) { + $scope.inspectorKey = selectedItem.getCapability("type").typeDef.inspector; + } } } diff --git a/platform/features/layout/bundle.js b/platform/features/layout/bundle.js index 25a3d81e29..d7bb565d08 100644 --- a/platform/features/layout/bundle.js +++ b/platform/features/layout/bundle.js @@ -272,7 +272,8 @@ define([ "$scope", "$q", "dialogService", - "openmct" + "openmct", + "$element" ] } ], diff --git a/platform/features/layout/res/templates/fixed.html b/platform/features/layout/res/templates/fixed.html index 46cefb1ff5..cf9b913037 100644 --- a/platform/features/layout/res/templates/fixed.html +++ b/platform/features/layout/res/templates/fixed.html @@ -23,7 +23,7 @@ ng-controller="FixedController as controller"> -
+
@@ -35,35 +35,28 @@
+ mct-selectable="controller.getContext(element)" + mct-init-select="controller.shouldSelect(element)"> - +
- - +
+ mct-drag-up="controller.endDrag()" + ng-style="controller.getSelectedElementStyle()">
+ mct-drag-up="controller.endDrag(handle)">
-
diff --git a/platform/features/layout/src/FixedController.js b/platform/features/layout/src/FixedController.js index 819fc26681..bb916d4ae8 100644 --- a/platform/features/layout/src/FixedController.js +++ b/platform/features/layout/src/FixedController.js @@ -47,7 +47,7 @@ define( * @constructor * @param {Scope} $scope the controller's Angular scope */ - function FixedController($scope, $q, dialogService, openmct) { + function FixedController($scope, $q, dialogService, openmct, $element) { this.names = {}; // Cache names by ID this.values = {}; // Cache values by ID this.elementProxiesById = {}; @@ -55,9 +55,11 @@ define( this.telemetryObjects = []; this.subscriptions = []; this.openmct = openmct; + this.$element = $element; this.$scope = $scope; this.gridSize = $scope.domainObject && $scope.domainObject.getModel().layoutGrid; + this.fixedViewSelectable = false; var self = this; [ @@ -87,9 +89,8 @@ define( // Update the style for a selected element function updateSelectionStyle() { - var element = self.selection && self.selection.get(); - if (element) { - element.style = convertPosition(element); + if (self.selectedElementProxy) { + self.selectedElementProxy.style = convertPosition(self.selectedElementProxy); } } @@ -136,25 +137,19 @@ define( // Decorate elements in the current configuration function refreshElements() { - // Cache selection; we are instantiating new proxies - // so we may want to restore this. - var selected = self.selection && self.selection.get(), - elements = (($scope.configuration || {}).elements || []), - index = -1; // Start with a 'not-found' value - - // Find the selection in the new array - if (selected !== undefined) { - index = elements.indexOf(selected.element); - } + var elements = (($scope.configuration || {}).elements || []); // Create the new proxies... self.elementProxies = elements.map(makeProxyElement); - // Clear old selection, and restore if appropriate - if (self.selection) { - self.selection.deselect(); - if (index > -1) { - self.select(self.elementProxies[index]); + // If selection is not in array, select parent. + // Otherwise, set the element to select after refresh. + if (self.selectedElementProxy) { + var index = elements.indexOf(self.selectedElementProxy.element); + if (index === -1) { + self.$element[0].click(); + } else if (!self.elementToSelectAfterRefresh) { + self.elementToSelectAfterRefresh = self.elementProxies[index].element; } } @@ -224,12 +219,12 @@ define( $scope.configuration.elements || []; // Store the position of this element. $scope.configuration.elements.push(element); + + self.elementToSelectAfterRefresh = element; + // Refresh displayed elements refreshElements(); - // Select the newly-added element - self.select( - self.elementProxies[self.elementProxies.length - 1] - ); + // Mark change as persistable if ($scope.commit) { $scope.commit("Dropped an element."); @@ -263,21 +258,36 @@ define( self.getTelemetry($scope.domainObject); } + // Sets the selectable object in response to the selection change event. + function setSelection(selectable) { + var selection = selectable[0]; + + if (!selection) { + return; + } + + if (selection.context.elementProxy) { + self.selectedElementProxy = selection.context.elementProxy; + self.mvHandle = self.generateDragHandle(self.selectedElementProxy); + self.resizeHandles = self.generateDragHandles(self.selectedElementProxy); + } else { + // Make fixed view selectable if it's not already. + if (!self.fixedViewSelectable) { + self.fixedViewSelectable = true; + selection.context.viewProxy = new FixedProxy(addElement, $q, dialogService); + self.openmct.selection.select(selection); + } + + self.resizeHandles = []; + self.mvHandle = undefined; + self.selectedElementProxy = undefined; + } + } + this.elementProxies = []; this.generateDragHandle = generateDragHandle; this.generateDragHandles = generateDragHandles; - - // Track current selection state - $scope.$watch("selection", function (selection) { - this.selection = selection; - - // Expose the view's selection proxy - if (this.selection) { - this.selection.proxy( - new FixedProxy(addElement, $q, dialogService) - ); - } - }.bind(this)); + this.updateSelectionStyle = updateSelectionStyle; // Detect changes to grid size $scope.$watch("model.layoutGrid", updateElementPositions); @@ -298,10 +308,13 @@ define( $scope.$on("$destroy", function () { self.unsubscribe(); self.openmct.time.off("bounds", updateDisplayBounds); + self.openmct.selection.off("change", setSelection); }); // Respond to external bounds changes this.openmct.time.on("bounds", updateDisplayBounds); + this.openmct.selection.on('change', setSelection); + this.$element.on('click', this.bypassSelection.bind(this)); } /** @@ -492,42 +505,56 @@ define( }; /** - * Check if the element is currently selected, or (if no - * argument is supplied) get the currently selected element. - * @returns {boolean} true if selected + * Checks if the element should be selected or not. + * + * @param elementProxy the element to check + * @returns {boolean} true if the element should be selected. */ - FixedController.prototype.selected = function (element) { - var selection = this.selection; - return selection && ((arguments.length > 0) ? - selection.selected(element) : selection.get()); - }; - - /** - * Set the active user selection in this view. - * @param element the element to select - */ - FixedController.prototype.select = function select(element, event) { - if (event) { - event.stopPropagation(); - } - - if (this.selection) { - // Update selection... - this.selection.select(element); - // ...as well as move, resize handles - this.mvHandle = this.generateDragHandle(element); - this.resizeHandles = this.generateDragHandles(element); + FixedController.prototype.shouldSelect = function (elementProxy) { + if (elementProxy.element === this.elementToSelectAfterRefresh) { + delete this.elementToSelectAfterRefresh; + return true; + } else { + return false; } }; /** - * Clear the current user selection. + * Checks if an element is currently selected. + * + * @returns {boolean} true if an element is selected. */ - FixedController.prototype.clearSelection = function () { - if (this.selection) { - this.selection.deselect(); - this.resizeHandles = []; - this.mvHandle = undefined; + FixedController.prototype.isElementSelected = function () { + return (this.selectedElementProxy) ? true : false; + }; + + /** + * Gets the style for the selected element. + * + * @returns {string} element style + */ + FixedController.prototype.getSelectedElementStyle = function () { + return (this.selectedElementProxy) ? this.selectedElementProxy.style : undefined; + }; + + /** + * Gets the selected element. + * + * @returns the selected element + */ + FixedController.prototype.getSelectedElement = function () { + return this.selectedElementProxy; + }; + + /** + * Prevents the event from bubbling up if drag is in progress. + */ + FixedController.prototype.bypassSelection = function ($event) { + if (this.dragInProgress) { + if ($event) { + $event.stopPropagation(); + } + return; } }; @@ -548,6 +575,38 @@ define( return this.mvHandle; }; + /** + * Gets the selection context. + * + * @param elementProxy the element proxy + * @returns {object} the context object which includes elementProxy and toolbar + */ + FixedController.prototype.getContext = function (elementProxy) { + return { + elementProxy: elementProxy, + toolbar: elementProxy + }; + }; + + /** + * End drag. + * + * @param handle the resize handle + */ + FixedController.prototype.endDrag = function (handle) { + this.dragInProgress = true; + + setTimeout(function () { + this.dragInProgress = false; + }.bind(this), 0); + + if (handle) { + handle.endDrag(); + } else { + this.moveHandle().endDrag(); + } + }; + return FixedController; } ); diff --git a/platform/features/layout/src/FixedDragHandle.js b/platform/features/layout/src/FixedDragHandle.js index 7ff5fb0576..ac48cbd7f9 100644 --- a/platform/features/layout/src/FixedDragHandle.js +++ b/platform/features/layout/src/FixedDragHandle.js @@ -65,7 +65,7 @@ define( * Start a drag gesture. This should be called when a drag * begins to track initial state. */ - FixedDragHandle.prototype.startDrag = function startDrag() { + FixedDragHandle.prototype.startDrag = function () { // Cache initial x/y positions this.dragging = { x: this.elementHandle.x(), diff --git a/platform/features/layout/src/elements/ElementProxy.js b/platform/features/layout/src/elements/ElementProxy.js index cab7970a41..78c166b390 100644 --- a/platform/features/layout/src/elements/ElementProxy.js +++ b/platform/features/layout/src/elements/ElementProxy.js @@ -55,8 +55,8 @@ define( * @param element the fixed position element, as stored in its * configuration * @param index the element's index within its array - * @param {number[]} gridSize the current layout grid size in [x,y] from * @param {Array} elements the full array of elements + * @param {number[]} gridSize the current layout grid size in [x,y] from */ function ElementProxy(element, index, elements, gridSize) { /** diff --git a/platform/features/layout/test/FixedControllerSpec.js b/platform/features/layout/test/FixedControllerSpec.js index 8f42d98fe0..8db4557aa2 100644 --- a/platform/features/layout/test/FixedControllerSpec.js +++ b/platform/features/layout/test/FixedControllerSpec.js @@ -21,8 +21,14 @@ *****************************************************************************/ define( - ["../src/FixedController"], - function (FixedController) { + [ + "../src/FixedController", + "zepto" + ], + function ( + FixedController, + $ + ) { describe("The Fixed Position controller", function () { var mockScope, @@ -46,6 +52,9 @@ define( mockMetadata, mockTimeSystem, mockLimitEvaluator, + mockSelection, + $element = [], + selectable = [], controller; // Utility function; find a watch for a given expression @@ -180,17 +189,30 @@ define( mockScope.model = testModel; mockScope.configuration = testConfiguration; - mockScope.selection = jasmine.createSpyObj( - 'selection', - ['select', 'get', 'selected', 'deselect', 'proxy'] - ); + + selectable[0] = { + context: { + oldItem: mockDomainObject + } + }; + mockSelection = jasmine.createSpyObj("selection", [ + 'select', + 'on', + 'off', + 'get' + ]); + mockSelection.get.andCallThrough(); mockOpenMCT = { time: mockConductor, telemetry: mockTelemetryAPI, - composition: mockCompositionAPI + composition: mockCompositionAPI, + selection: mockSelection }; + $element = $('
'); + spyOn($element[0], 'click'); + mockMetadata = jasmine.createSpyObj('mockMetadata', [ 'valuesForHints', 'value', @@ -226,11 +248,11 @@ define( mockScope, mockQ, mockDialogService, - mockOpenMCT + mockOpenMCT, + $element ); findWatch("model.layoutGrid")(testModel.layoutGrid); - findWatch("selection")(mockScope.selection); }); it("subscribes when a domain object is available", function () { @@ -306,41 +328,41 @@ define( }); it("allows elements to be selected", function () { - var elements; - testModel.modified = 1; findWatch("model.modified")(testModel.modified); - elements = controller.getElements(); - controller.select(elements[1]); - expect(mockScope.selection.select) - .toHaveBeenCalledWith(elements[1]); + selectable[0].context.elementProxy = controller.getElements()[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); + + expect(controller.isElementSelected()).toBe(true); }); it("allows selection retrieval", function () { - // selected with no arguments should give the current - // selection var elements; testModel.modified = 1; findWatch("model.modified")(testModel.modified); elements = controller.getElements(); - controller.select(elements[1]); - mockScope.selection.get.andReturn(elements[1]); - expect(controller.selected()).toEqual(elements[1]); + selectable[0].context.elementProxy = elements[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); + + expect(controller.getSelectedElement()).toEqual(elements[1]); }); - it("allows selections to be cleared", function () { - var elements; - + it("selects the parent view when selected element is removed", function () { testModel.modified = 1; findWatch("model.modified")(testModel.modified); - elements = controller.getElements(); - controller.select(elements[1]); - controller.clearSelection(); - expect(controller.selected(elements[1])).toBeFalsy(); + var elements = controller.getElements(); + selectable[0].context.elementProxy = elements[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); + + elements[1].remove(); + testModel.modified = 2; + findWatch("model.modified")(testModel.modified); + + expect($element[0].click).toHaveBeenCalled(); }); it("retains selections during refresh", function () { @@ -352,23 +374,21 @@ define( findWatch("model.modified")(testModel.modified); elements = controller.getElements(); - controller.select(elements[1]); + selectable[0].context.elementProxy = elements[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); - // Verify precondition - expect(mockScope.selection.select.calls.length).toEqual(1); - - // Mimic selection behavior - mockScope.selection.get.andReturn(elements[1]); + expect(controller.getSelectedElement()).toEqual(elements[1]); elements[2].remove(); testModel.modified = 2; findWatch("model.modified")(testModel.modified); elements = controller.getElements(); + // Verify removal, as test assumes this expect(elements.length).toEqual(2); - expect(mockScope.selection.select.calls.length).toEqual(2); + expect(controller.shouldSelect(elements[1])).toBe(true); }); it("Displays received values for telemetry elements", function () { @@ -505,21 +525,25 @@ define( }); it("exposes a view-level selection proxy", function () { - expect(mockScope.selection.proxy).toHaveBeenCalledWith( - jasmine.any(Object) - ); + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); + var selection = mockOpenMCT.selection.select.mostRecentCall.args[0]; + + expect(mockOpenMCT.selection.select).toHaveBeenCalled(); + expect(selection.context.viewProxy).toBeDefined(); }); it("exposes drag handles", function () { var handles; - // Select something so that drag handles are expected testModel.modified = 1; findWatch("model.modified")(testModel.modified); - controller.select(controller.getElements()[1]); + + selectable[0].context.elementProxy = controller.getElements()[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); // Should have a non-empty array of handles handles = controller.handles(); + expect(handles).toEqual(jasmine.any(Array)); expect(handles.length).not.toEqual(0); @@ -532,15 +556,14 @@ define( }); it("exposes a move handle", function () { - var handle; - - // Select something so that drag handles are expected testModel.modified = 1; findWatch("model.modified")(testModel.modified); - controller.select(controller.getElements()[1]); + + selectable[0].context.elementProxy = controller.getElements()[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); // Should have a move handle - handle = controller.moveHandle(); + var handle = controller.moveHandle(); // And it should have start/continue/end drag methods expect(handle.startDrag).toEqual(jasmine.any(Function)); @@ -551,26 +574,40 @@ define( it("updates selection style during drag", function () { var oldStyle; - // Select something so that drag handles are expected testModel.modified = 1; findWatch("model.modified")(testModel.modified); - controller.select(controller.getElements()[1]); - mockScope.selection.get.andReturn(controller.getElements()[1]); + + selectable[0].context.elementProxy = controller.getElements()[1]; + mockOpenMCT.selection.on.mostRecentCall.args[1](selectable); // Get style - oldStyle = controller.selected().style; + oldStyle = controller.getSelectedElementStyle(); // Start a drag gesture controller.moveHandle().startDrag(); // Haven't moved yet; style shouldn't have updated yet - expect(controller.selected().style).toEqual(oldStyle); + expect(controller.getSelectedElementStyle()).toEqual(oldStyle); // Drag a little controller.moveHandle().continueDrag([1000, 100]); // Style should have been updated - expect(controller.selected().style).not.toEqual(oldStyle); + expect(controller.getSelectedElementStyle()).not.toEqual(oldStyle); + }); + + it("cleans up slection on scope destroy", function () { + expect(mockScope.$on).toHaveBeenCalledWith( + '$destroy', + jasmine.any(Function) + ); + + mockScope.$on.mostRecentCall.args[1](); + + expect(mockOpenMCT.selection.off).toHaveBeenCalledWith( + 'change', + jasmine.any(Function) + ); }); describe("on display bounds changes", function () { @@ -702,6 +739,14 @@ define( expect(controller.getElements()[0].cssClass).toEqual("alarm-a"); }); }); + + it("listens for selection change events", function () { + expect(mockOpenMCT.selection.on).toHaveBeenCalledWith( + 'change', + jasmine.any(Function) + ); + }); + }); }); } diff --git a/src/selection/Selection.js b/src/selection/Selection.js index e3d7882f14..1a7fd3bed5 100644 --- a/src/selection/Selection.js +++ b/src/selection/Selection.js @@ -115,7 +115,7 @@ define(['EventEmitter'], function (EventEmitter) { element.addEventListener('click', selectCapture); if (select) { - this.select(selectable); + element.click(); } return function () {