From e4aaa860a3a5c3509dc11174cbebdeaf02a3bf6b Mon Sep 17 00:00:00 2001 From: Deep Tailor Date: Tue, 29 Aug 2017 10:15:29 -0700 Subject: [PATCH 1/5] update mct-split-pane to use userPreferenceWidth only when alias is provided, otherwise set position as usual (fix for timeline sync issue) --- .../general/src/directives/MCTSplitPane.js | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/platform/commonUI/general/src/directives/MCTSplitPane.js b/platform/commonUI/general/src/directives/MCTSplitPane.js index f52876335c..00daca64d7 100644 --- a/platform/commonUI/general/src/directives/MCTSplitPane.js +++ b/platform/commonUI/general/src/directives/MCTSplitPane.js @@ -100,7 +100,7 @@ define( anchor, activeInterval, position, - splitterSize, + splitterSize, alias = $attrs.alias !== undefined ? "mctSplitPane-" + $attrs.alias : undefined, @@ -108,7 +108,7 @@ define( //convert string to number from localStorage userWidthPreference = $window.localStorage.getItem(alias) === null ? undefined : Number($window.localStorage.getItem(alias)); - + // Get relevant size (height or width) of DOM element function getSize(domElement) { return (anchor.orientation === 'vertical' ? @@ -117,6 +117,10 @@ define( // Apply styles to child elements function updateChildren(children) { + if (alias) { + position = userWidthPreference || position; + } + // Pick out correct elements to update, flowing from // selected anchor edge. var first = children.eq(anchor.reversed ? 2 : 0), @@ -126,7 +130,7 @@ define( splitterSize = getSize(splitter[0]); first.css(anchor.edge, "0px"); - first.css(anchor.dimension, (userWidthPreference || position) + 'px'); + first.css(anchor.dimension, position + 'px'); // Get actual size (to obey min-width etc.) firstSize = getSize(first[0]); @@ -135,8 +139,8 @@ define( splitter.css(anchor.opposite, "auto"); last.css(anchor.edge, firstSize + splitterSize + 'px'); - last.css(anchor.opposite, "0px"); - position = firstSize + splitterSize; + last.css(anchor.opposite, '0px'); + position = firstSize; } // Update positioning of contained elements @@ -162,28 +166,27 @@ define( // Getter-setter for the pixel offset of the splitter, // relative to the current edge. function getSetPosition(value) { - var prior = position; if (typeof value === 'number') { position = value; enforceExtrema(); updateElementPositions(); // Pass change up so this state can be shared - if (positionParsed.assign && position !== prior) { + if (positionParsed.assign) { positionParsed.assign($scope, position); } - } + } + return position; } function setUserWidthPreference(value) { - userWidthPreference = value - splitterSize; + userWidthPreference = value; } function persistToLocalStorage(value) { if (alias) { - userWidthPreference = value - splitterSize; - $window.localStorage.setItem(alias, userWidthPreference); + $window.localStorage.setItem(alias, value); } } @@ -218,20 +221,21 @@ define( $scope.$on('$destroy', function () { $interval.cancel(activeInterval); }); - + // Interface exposed by controller, for mct-splitter to user return { anchor: function () { return anchor; }, - position: function (value) { - if (arguments.length > 0) { - setUserWidthPreference(value); - return getSetPosition(value); - } else { + position: function (initialValue, newValue) { + if(arguments.length === 0){ return getSetPosition(); } + if (initialValue !== newValue) { + setUserWidthPreference(newValue); + getSetPosition(newValue); + } }, startResizing: function () { toggleClass('resizing'); @@ -253,4 +257,4 @@ define( return MCTSplitPane; } -); +); \ No newline at end of file From 556296096daa4229b1846f42c06ca4939c9a9833 Mon Sep 17 00:00:00 2001 From: Deep Tailor Date: Tue, 29 Aug 2017 10:34:44 -0700 Subject: [PATCH 2/5] fix tests to correspond with changes made to MCTSplitPane --- .../general/test/directives/MCTSplitPaneSpec.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js b/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js index abd0935021..2417ccd2b9 100644 --- a/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js +++ b/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js @@ -140,7 +140,7 @@ define( it("exposes its splitter's initial position", function () { expect(controller.position()).toEqual( - mockFirstPane[0].offsetWidth + mockSplitter[0].offsetWidth + mockFirstPane[0].offsetWidth ); }); @@ -164,11 +164,12 @@ define( }); it("allows positions to be set", function () { + var intitialValue = mockChildren[0].offsetWidth; var testValue = mockChildren[0].offsetWidth + 50; - controller.position(testValue); + controller.position(intitialValue, testValue); expect(mockFirstPane.css).toHaveBeenCalledWith( 'width', - (testValue - mockSplitter[0].offsetWidth) + 'px' + (testValue) + 'px' ); }); @@ -200,11 +201,11 @@ define( mockFirstPane[0].offsetWidth += 100; // Should not reflect the change yet expect(controller.position()).not.toEqual( - mockFirstPane[0].offsetWidth + mockSplitter[0].offsetWidth + mockFirstPane[0].offsetWidth ); mockInterval.mostRecentCall.args[0](); expect(controller.position()).toEqual( - mockFirstPane[0].offsetWidth + mockSplitter[0].offsetWidth + mockFirstPane[0].offsetWidth ); }); @@ -216,7 +217,7 @@ define( it("saves user preference to localStorage when user is done resizing", function () { controller.endResizing(100); - expect(Number(mockWindow.localStorage.getItem('mctSplitPane-rightSide'))).toEqual(100 - mockSplitter[0].offsetWidth); + expect(Number(mockWindow.localStorage.getItem('mctSplitPane-rightSide'))).toEqual(100); }); }); From aa336dfd57932ccf658181f5460127aaa313fc6b Mon Sep 17 00:00:00 2001 From: Deep Tailor Date: Tue, 29 Aug 2017 10:45:41 -0700 Subject: [PATCH 3/5] fix gulp checkstyle errors --- .../general/src/directives/MCTSplitPane.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/platform/commonUI/general/src/directives/MCTSplitPane.js b/platform/commonUI/general/src/directives/MCTSplitPane.js index 00daca64d7..cb845ed1bd 100644 --- a/platform/commonUI/general/src/directives/MCTSplitPane.js +++ b/platform/commonUI/general/src/directives/MCTSplitPane.js @@ -100,7 +100,7 @@ define( anchor, activeInterval, position, - splitterSize, + splitterSize, alias = $attrs.alias !== undefined ? "mctSplitPane-" + $attrs.alias : undefined, @@ -108,7 +108,7 @@ define( //convert string to number from localStorage userWidthPreference = $window.localStorage.getItem(alias) === null ? undefined : Number($window.localStorage.getItem(alias)); - + // Get relevant size (height or width) of DOM element function getSize(domElement) { return (anchor.orientation === 'vertical' ? @@ -119,8 +119,8 @@ define( function updateChildren(children) { if (alias) { position = userWidthPreference || position; - } - + } + // Pick out correct elements to update, flowing from // selected anchor edge. var first = children.eq(anchor.reversed ? 2 : 0), @@ -175,7 +175,7 @@ define( if (positionParsed.assign) { positionParsed.assign($scope, position); } - } + } return position; } @@ -221,7 +221,7 @@ define( $scope.$on('$destroy', function () { $interval.cancel(activeInterval); }); - + // Interface exposed by controller, for mct-splitter to user return { @@ -229,7 +229,7 @@ define( return anchor; }, position: function (initialValue, newValue) { - if(arguments.length === 0){ + if (arguments.length === 0) { return getSetPosition(); } if (initialValue !== newValue) { @@ -257,4 +257,4 @@ define( return MCTSplitPane; } -); \ No newline at end of file +); From 0e3b629d90ef545547108256fef362c82a3e80f5 Mon Sep 17 00:00:00 2001 From: Deep Tailor Date: Tue, 29 Aug 2017 11:27:01 -0700 Subject: [PATCH 4/5] update changes requested by victor and update corresponding tests --- platform/commonUI/general/src/directives/MCTSplitPane.js | 3 ++- platform/commonUI/general/src/directives/MCTSplitter.js | 2 +- platform/commonUI/general/test/directives/MCTSplitterSpec.js | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/platform/commonUI/general/src/directives/MCTSplitPane.js b/platform/commonUI/general/src/directives/MCTSplitPane.js index cb845ed1bd..0262d6f0a2 100644 --- a/platform/commonUI/general/src/directives/MCTSplitPane.js +++ b/platform/commonUI/general/src/directives/MCTSplitPane.js @@ -166,13 +166,14 @@ define( // Getter-setter for the pixel offset of the splitter, // relative to the current edge. function getSetPosition(value) { + var prior = position; if (typeof value === 'number') { position = value; enforceExtrema(); updateElementPositions(); // Pass change up so this state can be shared - if (positionParsed.assign) { + if (positionParsed.assign && position !== prior) { positionParsed.assign($scope, position); } } diff --git a/platform/commonUI/general/src/directives/MCTSplitter.js b/platform/commonUI/general/src/directives/MCTSplitter.js index 69db2f0a02..315a00fad2 100644 --- a/platform/commonUI/general/src/directives/MCTSplitter.js +++ b/platform/commonUI/general/src/directives/MCTSplitter.js @@ -57,7 +57,7 @@ define( // Update the position of this splitter newPosition = initialPosition + pixelDelta; - mctSplitPane.position(newPosition); + mctSplitPane.position(initialPosition, newPosition); }, // Grab the event when the user is done moving // the splitter and pass it on diff --git a/platform/commonUI/general/test/directives/MCTSplitterSpec.js b/platform/commonUI/general/test/directives/MCTSplitterSpec.js index caf0fd468d..c0550d0975 100644 --- a/platform/commonUI/general/test/directives/MCTSplitterSpec.js +++ b/platform/commonUI/general/test/directives/MCTSplitterSpec.js @@ -94,7 +94,7 @@ define( it("repositions during drag", function () { mockScope.splitter.move([10, 0]); expect(mockSplitPane.position) - .toHaveBeenCalledWith(testPosition + 10); + .toHaveBeenCalledWith(testPosition, testPosition + 10); }); it("tell's the splitter when it is done resizing", function () { From e53b34ed60c21d9ae040f9cef66a49587190adb1 Mon Sep 17 00:00:00 2001 From: Deep Tailor Date: Tue, 29 Aug 2017 11:50:20 -0700 Subject: [PATCH 5/5] move newPosition check from mctSplitPane to splitter --- platform/commonUI/general/src/directives/MCTSplitPane.js | 9 ++++----- platform/commonUI/general/src/directives/MCTSplitter.js | 5 ++++- .../commonUI/general/test/directives/MCTSplitPaneSpec.js | 3 +-- .../commonUI/general/test/directives/MCTSplitterSpec.js | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/platform/commonUI/general/src/directives/MCTSplitPane.js b/platform/commonUI/general/src/directives/MCTSplitPane.js index 0262d6f0a2..f84c67a0a3 100644 --- a/platform/commonUI/general/src/directives/MCTSplitPane.js +++ b/platform/commonUI/general/src/directives/MCTSplitPane.js @@ -229,14 +229,13 @@ define( anchor: function () { return anchor; }, - position: function (initialValue, newValue) { + position: function (newPosition) { if (arguments.length === 0) { return getSetPosition(); } - if (initialValue !== newValue) { - setUserWidthPreference(newValue); - getSetPosition(newValue); - } + + setUserWidthPreference(newPosition); + return getSetPosition(newPosition); }, startResizing: function () { toggleClass('resizing'); diff --git a/platform/commonUI/general/src/directives/MCTSplitter.js b/platform/commonUI/general/src/directives/MCTSplitter.js index 315a00fad2..f0e4c69840 100644 --- a/platform/commonUI/general/src/directives/MCTSplitter.js +++ b/platform/commonUI/general/src/directives/MCTSplitter.js @@ -57,7 +57,10 @@ define( // Update the position of this splitter newPosition = initialPosition + pixelDelta; - mctSplitPane.position(initialPosition, newPosition); + + if (initialPosition !== newPosition) { + mctSplitPane.position(newPosition); + } }, // Grab the event when the user is done moving // the splitter and pass it on diff --git a/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js b/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js index 2417ccd2b9..51ebc48244 100644 --- a/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js +++ b/platform/commonUI/general/test/directives/MCTSplitPaneSpec.js @@ -164,9 +164,8 @@ define( }); it("allows positions to be set", function () { - var intitialValue = mockChildren[0].offsetWidth; var testValue = mockChildren[0].offsetWidth + 50; - controller.position(intitialValue, testValue); + controller.position(testValue); expect(mockFirstPane.css).toHaveBeenCalledWith( 'width', (testValue) + 'px' diff --git a/platform/commonUI/general/test/directives/MCTSplitterSpec.js b/platform/commonUI/general/test/directives/MCTSplitterSpec.js index c0550d0975..caf0fd468d 100644 --- a/platform/commonUI/general/test/directives/MCTSplitterSpec.js +++ b/platform/commonUI/general/test/directives/MCTSplitterSpec.js @@ -94,7 +94,7 @@ define( it("repositions during drag", function () { mockScope.splitter.move([10, 0]); expect(mockSplitPane.position) - .toHaveBeenCalledWith(testPosition, testPosition + 10); + .toHaveBeenCalledWith(testPosition + 10); }); it("tell's the splitter when it is done resizing", function () {