From f11eced6b90a87cc074e8b2c34360f0efd1fce1b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Feb 2015 15:46:36 -0800 Subject: [PATCH 1/6] [Common UI] Add spec for MCT scroll directive parent Add spec for MCTScroll, which will act as a parent class for mct-scroll-x and mct-scroll-y directives, WTD-920. --- .../general/src/directives/MCTScroll.js | 0 .../general/test/directives/MCTScrollSpec.js | 90 +++++++++++++++++++ platform/commonUI/general/test/suite.json | 1 + 3 files changed, 91 insertions(+) create mode 100644 platform/commonUI/general/src/directives/MCTScroll.js create mode 100644 platform/commonUI/general/test/directives/MCTScrollSpec.js diff --git a/platform/commonUI/general/src/directives/MCTScroll.js b/platform/commonUI/general/src/directives/MCTScroll.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/platform/commonUI/general/test/directives/MCTScrollSpec.js b/platform/commonUI/general/test/directives/MCTScrollSpec.js new file mode 100644 index 0000000000..7b5dfff77b --- /dev/null +++ b/platform/commonUI/general/test/directives/MCTScrollSpec.js @@ -0,0 +1,90 @@ +/*global define,describe,it,expect,jasmine,beforeEach*/ +define( + ['../../src/directives/MCTScroll'], + function (MCTScroll) { + "use strict"; + + var EVENT_PROPERTY = "testProperty", + ATTRIBUTE = "testAttribute", + EXPRESSION = "some.expression"; + + + // MCTScroll is the commonality between mct-scroll-x and + // mct-scroll-y; it gets the event property to watch and + // the attribute which contains the associated assignable + // expression. + describe("An mct-scroll-* directive", function () { + var mockParse, + mockParsed, + mockScope, + mockElement, + testAttrs, + mctScroll; + + beforeEach(function () { + mockParse = jasmine.createSpy('$parse'); + mockParsed = jasmine.createSpy('parsed'); + mockParsed.assign = jasmine.createSpy('assign'); + + mockScope = jasmine.createSpyObj('$scope', ['$watch']); + mockElement = [{ scrollLeft: 42 }]; + mockElement.on = jasmine.createSpy('on'); + + mockParse.andReturn(mockParsed); + + testAttrs = {}; + testAttrs[ATTRIBUTE] = EXPRESSION; + + mctScroll = new MCTScroll( + mockParse, + EVENT_PROPERTY, + ATTRIBUTE + ); + mctScroll.link(mockScope, mockElement, testAttrs); + }); + + it("is available for attributes", function () { + expect(mctScroll.restrict).toEqual('A'); + }); + + it("does not create an isolate scope", function () { + expect(mctScroll.scope).toBeUndefined(); + }); + + it("watches for changes in observed expression", function () { + expect(mockScope.$watch).toHaveBeenCalledWith( + EXPRESSION, + jasmine.any(Function) + ); + // Should have been only watch (other tests need this to be true) + expect(mockScope.$watch.calls.length).toEqual(0); + }); + + it("listens for scroll events", function () { + expect(mockElement.on).toHaveBeenCalledWith( + 'scroll', + jasmine.any(Function) + ); + // Should have been only listener (other tests need this to be true) + expect(mockElement.on.calls.length).toEqual(0); + }); + + it("publishes initial scroll state", function () { + expect(mockParse).toHaveBeenCalledWith(EXPRESSION); + expect(mockParsed.assign).toHaveBeenCalledWith(42); + }); + + it("updates scroll state when scope changes", function () { + mockScope.$watch.mostRecentCall.args[1](64); + expect(mockElement[0].scrollLeft).toEqual(64); + }); + + it("updates scope when scroll state changes", function () { + mockElement[0].scrollLeft = 12321; + mockElement.on.mostRecentCall.args({ target: mockElement[0] }); + expect(mockParsed.assign).toHaveBeenCalledWith(12321); + }); + + }); + } +); \ No newline at end of file diff --git a/platform/commonUI/general/test/suite.json b/platform/commonUI/general/test/suite.json index ae3a54f37b..58d94a4d95 100644 --- a/platform/commonUI/general/test/suite.json +++ b/platform/commonUI/general/test/suite.json @@ -11,5 +11,6 @@ "directives/MCTContainer", "directives/MCTDrag", "directives/MCTResize", + "directives/MCTScroll", "StyleSheetLoader" ] \ No newline at end of file From f8b352c4d8f1855a8c2e5054424fa68b944e235f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Feb 2015 16:01:38 -0800 Subject: [PATCH 2/6] [Common UI] Implement mct-scroll-* parent Implement parent directive for mct-scroll-x, mct-scroll-y. WTD-920. --- .../general/src/directives/MCTScroll.js | 57 +++++++++++++++++++ .../general/test/directives/MCTScrollSpec.js | 12 ++-- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/general/src/directives/MCTScroll.js b/platform/commonUI/general/src/directives/MCTScroll.js index e69de29bb2..d9abf810a1 100644 --- a/platform/commonUI/general/src/directives/MCTScroll.js +++ b/platform/commonUI/general/src/directives/MCTScroll.js @@ -0,0 +1,57 @@ +/*global define*/ + +define( + [], + function () { + 'use strict'; + + /** + * Superclass of mct-scroll-x and mct-scroll-y directives. Listens + * for scroll events and publishes their results into scope; watches + * scope and updates scroll state to match. This varies for x- and y- + * directives only by the attribute name chosen to find the expression, + * and the property (scrollLeft or scrollTop) managed within the + * element. + * @constructor + * @param $parse Angular's $parse + * @param {string} property property to manage within the HTML element + * @param {string} attribute attribute to look at for the assignable + * Angular expression + */ + function MCTScroll($parse, property, attribute) { + function link(scope, element, attrs) { + var expr = attrs[attribute], + parsed = $parse(expr); + + // Set the element's scroll to match the scope's state + function updateElement(value) { + element[0][property] = value; + } + + // Handle event; assign to scroll state to scope + function updateScope() { + parsed.assign(element[0][property]); + } + + // Initialize state in scope + updateScope(); + + // Update element state when value in scope changes + scope.$watch(expr, updateElement); + + // Update state in scope when element is scrolled + element.on('scroll', updateScope); + } + + return { + // Restrict to attributes + restrict: "A", + // Use this link function + link: link + }; + } + + return MCTScroll; + + } +); \ No newline at end of file diff --git a/platform/commonUI/general/test/directives/MCTScrollSpec.js b/platform/commonUI/general/test/directives/MCTScrollSpec.js index 7b5dfff77b..8b14606d53 100644 --- a/platform/commonUI/general/test/directives/MCTScrollSpec.js +++ b/platform/commonUI/general/test/directives/MCTScrollSpec.js @@ -27,7 +27,7 @@ define( mockParsed.assign = jasmine.createSpy('assign'); mockScope = jasmine.createSpyObj('$scope', ['$watch']); - mockElement = [{ scrollLeft: 42 }]; + mockElement = [{ testProperty: 42 }]; mockElement.on = jasmine.createSpy('on'); mockParse.andReturn(mockParsed); @@ -57,7 +57,7 @@ define( jasmine.any(Function) ); // Should have been only watch (other tests need this to be true) - expect(mockScope.$watch.calls.length).toEqual(0); + expect(mockScope.$watch.calls.length).toEqual(1); }); it("listens for scroll events", function () { @@ -66,7 +66,7 @@ define( jasmine.any(Function) ); // Should have been only listener (other tests need this to be true) - expect(mockElement.on.calls.length).toEqual(0); + expect(mockElement.on.calls.length).toEqual(1); }); it("publishes initial scroll state", function () { @@ -76,12 +76,12 @@ define( it("updates scroll state when scope changes", function () { mockScope.$watch.mostRecentCall.args[1](64); - expect(mockElement[0].scrollLeft).toEqual(64); + expect(mockElement[0].testProperty).toEqual(64); }); it("updates scope when scroll state changes", function () { - mockElement[0].scrollLeft = 12321; - mockElement.on.mostRecentCall.args({ target: mockElement[0] }); + mockElement[0].testProperty = 12321; + mockElement.on.mostRecentCall.args[1]({ target: mockElement[0] }); expect(mockParsed.assign).toHaveBeenCalledWith(12321); }); From 9a88e5172ab572c9b1a40b0d69eceb08d729626b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Feb 2015 16:05:57 -0800 Subject: [PATCH 3/6] [Common UI] Expose mct-scroll-x, mct-scroll-y Expose mct-scroll-x and mct-scroll-y directives. WTD-920. --- platform/commonUI/general/bundle.json | 28 +++++++++++++++++++ .../general/src/directives/MCTScroll.js | 6 +++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/platform/commonUI/general/bundle.json b/platform/commonUI/general/bundle.json index f7b60b096d..5bf767d2d1 100644 --- a/platform/commonUI/general/bundle.json +++ b/platform/commonUI/general/bundle.json @@ -105,6 +105,34 @@ "key": "mctResize", "implementation": "directives/MCTResize.js", "depends": [ "$timeout" ] + }, + { + "key": "mctScrollX", + "implementations": "directives/MCTScroll.js", + "depends": [ "$parse", "MCT_SCROLL_X_PROPERTY", "MCT_SCROLL_X_ATTRIBUTE" ] + }, + { + "key": "mctScrollY", + "implementations": "directives/MCTScroll.js", + "depends": [ "$parse", "MCT_SCROLL_Y_PROPERTY", "MCT_SCROLL_Y_ATTRIBUTE" ] + } + ], + "constants": [ + { + "key": "MCT_SCROLL_X_PROPERTY", + "value": "scrollLeft" + }, + { + "key": "MCT_SCROLL_X_ATTRIBUTE", + "value": "mctScrollX" + }, + { + "key": "MCT_SCROLL_Y_PROPERTY", + "value": "scrollTop" + }, + { + "key": "MCT_SCROLL_Y_ATTRIBUTE", + "value": "mctScrollY" } ], "containers": [ diff --git a/platform/commonUI/general/src/directives/MCTScroll.js b/platform/commonUI/general/src/directives/MCTScroll.js index d9abf810a1..f5b7a5b159 100644 --- a/platform/commonUI/general/src/directives/MCTScroll.js +++ b/platform/commonUI/general/src/directives/MCTScroll.js @@ -6,12 +6,16 @@ define( 'use strict'; /** - * Superclass of mct-scroll-x and mct-scroll-y directives. Listens + * Implements `mct-scroll-x` and `mct-scroll-y` directives. Listens * for scroll events and publishes their results into scope; watches * scope and updates scroll state to match. This varies for x- and y- * directives only by the attribute name chosen to find the expression, * and the property (scrollLeft or scrollTop) managed within the * element. + * + * This is exposed as two directives in `bundle.json`; the difference + * is handled purely by parameterization. + * * @constructor * @param $parse Angular's $parse * @param {string} property property to manage within the HTML element From 9fa09e25f05058484b9408593d7451911c38d856 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Feb 2015 16:28:53 -0800 Subject: [PATCH 4/6] [Common UI] Tweak mct-scroll-* directives Make fixes/tweaks to behavior of mct-scroll-x and mct-scroll-y directives after testing them against timeline markup. WTD-920. --- platform/commonUI/general/bundle.json | 4 ++-- platform/commonUI/general/src/directives/MCTScroll.js | 3 ++- platform/commonUI/general/test/directives/MCTScrollSpec.js | 7 ++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/platform/commonUI/general/bundle.json b/platform/commonUI/general/bundle.json index 5bf767d2d1..bd94eaabdf 100644 --- a/platform/commonUI/general/bundle.json +++ b/platform/commonUI/general/bundle.json @@ -108,12 +108,12 @@ }, { "key": "mctScrollX", - "implementations": "directives/MCTScroll.js", + "implementation": "directives/MCTScroll.js", "depends": [ "$parse", "MCT_SCROLL_X_PROPERTY", "MCT_SCROLL_X_ATTRIBUTE" ] }, { "key": "mctScrollY", - "implementations": "directives/MCTScroll.js", + "implementation": "directives/MCTScroll.js", "depends": [ "$parse", "MCT_SCROLL_Y_PROPERTY", "MCT_SCROLL_Y_ATTRIBUTE" ] } ], diff --git a/platform/commonUI/general/src/directives/MCTScroll.js b/platform/commonUI/general/src/directives/MCTScroll.js index f5b7a5b159..6436bd3dcc 100644 --- a/platform/commonUI/general/src/directives/MCTScroll.js +++ b/platform/commonUI/general/src/directives/MCTScroll.js @@ -34,7 +34,8 @@ define( // Handle event; assign to scroll state to scope function updateScope() { - parsed.assign(element[0][property]); + parsed.assign(scope, element[0][property]); + scope.$apply(expr); } // Initialize state in scope diff --git a/platform/commonUI/general/test/directives/MCTScrollSpec.js b/platform/commonUI/general/test/directives/MCTScrollSpec.js index 8b14606d53..200c418798 100644 --- a/platform/commonUI/general/test/directives/MCTScrollSpec.js +++ b/platform/commonUI/general/test/directives/MCTScrollSpec.js @@ -26,7 +26,7 @@ define( mockParsed = jasmine.createSpy('parsed'); mockParsed.assign = jasmine.createSpy('assign'); - mockScope = jasmine.createSpyObj('$scope', ['$watch']); + mockScope = jasmine.createSpyObj('$scope', ['$watch', '$apply']); mockElement = [{ testProperty: 42 }]; mockElement.on = jasmine.createSpy('on'); @@ -71,7 +71,7 @@ define( it("publishes initial scroll state", function () { expect(mockParse).toHaveBeenCalledWith(EXPRESSION); - expect(mockParsed.assign).toHaveBeenCalledWith(42); + expect(mockParsed.assign).toHaveBeenCalledWith(mockScope, 42); }); it("updates scroll state when scope changes", function () { @@ -82,7 +82,8 @@ define( it("updates scope when scroll state changes", function () { mockElement[0].testProperty = 12321; mockElement.on.mostRecentCall.args[1]({ target: mockElement[0] }); - expect(mockParsed.assign).toHaveBeenCalledWith(12321); + expect(mockParsed.assign).toHaveBeenCalledWith(mockScope, 12321); + expect(mockScope.$apply).toHaveBeenCalledWith(EXPRESSION); }); }); From 5df271072170b1f4be83e53d89c0c05555afe952 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Feb 2015 16:34:51 -0800 Subject: [PATCH 5/6] [Common UI] Document mct-scroll-* directives Add documentation for mct-scroll-x and mct-scroll-y, WTD-920. --- platform/commonUI/general/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 platform/commonUI/general/README.md diff --git a/platform/commonUI/general/README.md b/platform/commonUI/general/README.md new file mode 100644 index 0000000000..817d99d4e9 --- /dev/null +++ b/platform/commonUI/general/README.md @@ -0,0 +1,8 @@ +# Directives + +* `mct-scroll-x` is an attribute whose value is an assignable + Angular expression. This two-way binds that expression to the + horizontal scroll state of the element on which it is applied. +* `mct-scroll-y` is an attribute whose value is an assignable + Angular expression. This two-way binds that expression to the + vertical scroll state of the element on which it is applied. \ No newline at end of file From 86ee28740f243d1b1105eef923b1ebf65d48d21f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 3 Mar 2015 13:50:39 -0800 Subject: [PATCH 6/6] [Common UI] Avoid infinite digest Avoid infinite digest when first applicy mct-scroll directives, WTD-920. --- platform/commonUI/general/src/directives/MCTScroll.js | 2 +- platform/commonUI/general/test/directives/MCTScrollSpec.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/platform/commonUI/general/src/directives/MCTScroll.js b/platform/commonUI/general/src/directives/MCTScroll.js index 6436bd3dcc..c0f0dbfcc5 100644 --- a/platform/commonUI/general/src/directives/MCTScroll.js +++ b/platform/commonUI/general/src/directives/MCTScroll.js @@ -39,7 +39,7 @@ define( } // Initialize state in scope - updateScope(); + parsed.assign(scope, element[0][property]); // Update element state when value in scope changes scope.$watch(expr, updateElement); diff --git a/platform/commonUI/general/test/directives/MCTScrollSpec.js b/platform/commonUI/general/test/directives/MCTScrollSpec.js index 200c418798..bf55d90aca 100644 --- a/platform/commonUI/general/test/directives/MCTScrollSpec.js +++ b/platform/commonUI/general/test/directives/MCTScrollSpec.js @@ -86,6 +86,11 @@ define( expect(mockScope.$apply).toHaveBeenCalledWith(EXPRESSION); }); + // This would trigger an infinite digest exception + it("does not call $apply during construction", function () { + expect(mockScope.$apply).not.toHaveBeenCalled(); + }); + }); } ); \ No newline at end of file