From f74199e60f1b7fb3582754ba7277952fa9e0989f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 20 May 2015 14:02:53 -0700 Subject: [PATCH 1/4] [Layout] Add check to view in frame Add check to template for views in frame to avoid populating frame prematurely; WTD-1182. --- platform/features/layout/res/templates/frame.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/features/layout/res/templates/frame.html b/platform/features/layout/res/templates/frame.html index cf4149947f..bb3a61ab56 100644 --- a/platform/features/layout/res/templates/frame.html +++ b/platform/features/layout/res/templates/frame.html @@ -14,7 +14,7 @@
+ mct-object="representation.selected.key && domainObject">
\ No newline at end of file From 9b6d8cf1ec0ccfce3168b61a2d01a642a9290f5b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 20 May 2015 16:26:28 -0700 Subject: [PATCH 2/4] [Representation] Minimize scope reuse When switching among domain objects and/or views, avoid reusing the same information is scope. The wrong information in scope can cause various failures in views, such as WTD-1182. --- .../features/layout/src/LayoutController.js | 3 ++ .../representation/src/MCTRepresentation.js | 34 +++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/platform/features/layout/src/LayoutController.js b/platform/features/layout/src/LayoutController.js index c8e057d0f8..f3d44110b5 100644 --- a/platform/features/layout/src/LayoutController.js +++ b/platform/features/layout/src/LayoutController.js @@ -69,6 +69,9 @@ define( function lookupPanels(ids) { var configuration = $scope.configuration || {}; + // Ensure ids is array-like + ids = ids || []; + // Pull panel positions from configuration rawPositions = shallowCopy(configuration.panels || {}, ids); diff --git a/platform/representation/src/MCTRepresentation.js b/platform/representation/src/MCTRepresentation.js index 4d44d39c7a..7381ce653c 100644 --- a/platform/representation/src/MCTRepresentation.js +++ b/platform/representation/src/MCTRepresentation.js @@ -72,15 +72,18 @@ define( function link($scope, element, attrs) { var activeRepresenters = representers.map(function (Representer) { - return new Representer($scope, element, attrs); - }); + return new Representer($scope, element, attrs); + }), + toClear = [], // Properties to clear out of scope on change + counter = 0; // Populate scope with any capabilities indicated by the // representation's extension definition function refreshCapabilities() { var domainObject = $scope.domainObject, representation = lookup($scope.key, domainObject), - uses = ((representation || {}).uses || []); + uses = ((representation || {}).uses || []), + myCounter = counter; if (domainObject) { // Update model @@ -94,10 +97,16 @@ define( " for representation ", $scope.key ].join("")); + $q.when( domainObject.useCapability(used) ).then(function (c) { - $scope[used] = c; + // Avoid clobbering capabilities from + // subsequent representations; + // Angular reuses scopes. + if (counter === myCounter) { + $scope[used] = c; + } }); }); } @@ -109,8 +118,7 @@ define( function refresh() { var domainObject = $scope.domainObject, representation = lookup($scope.key, domainObject), - uses = ((representation || {}).uses || []), - gestureKeys = ((representation || {}).gestures || []); + uses = ((representation || {}).uses || []); // Create an empty object named "representation", for this // representation to store local variables into. @@ -131,9 +139,19 @@ define( $log.warn("No representation found for " + $scope.key); } + // Clear out the scope from the last representation + toClear.forEach(function (property) { + delete $scope[property]; + }); + // Populate scope with fields associated with the current // domain object (if one has been passed in) if (domainObject) { + // Track how many representations we've made in this scope, + // to ensure that the correct representations are matched to + // the correct object/key pairs. + counter += 1; + // Initialize any capabilities refreshCapabilities(); @@ -147,6 +165,10 @@ define( activeRepresenters.forEach(function (representer) { representer.represent(representation, domainObject); }); + + // Track which properties we want to clear from scope + // next change object/key pair changes + toClear = uses.concat(['model']); } } From cde173dbdc8afc5c2f69a956cc8e2bc24a59ce18 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 20 May 2015 16:39:49 -0700 Subject: [PATCH 3/4] [Representation] Add test for scope clearing Add test clearing to verify that scope gets cleared on view changes, WTD-1182. --- .../test/MCTRepresentationSpec.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/platform/representation/test/MCTRepresentationSpec.js b/platform/representation/test/MCTRepresentationSpec.js index 835c40b71b..45e88daca5 100644 --- a/platform/representation/test/MCTRepresentationSpec.js +++ b/platform/representation/test/MCTRepresentationSpec.js @@ -160,6 +160,25 @@ define( // Should have gotten a warning - that's an unknown key expect(mockLog.warn).toHaveBeenCalled(); }); + + it("clears out obsolete peroperties from scope", function () { + mctRepresentation.link(mockScope, mockElement); + + mockScope.key = "def"; + mockScope.domainObject = mockDomainObject; + mockDomainObject.useCapability.andReturn("some value"); + + // Trigger the watch + mockScope.$watch.calls[0].args[1](); + expect(mockScope.testCapability).toBeDefined(); + + // Change the view + mockScope.key = "xyz"; + + // Trigger the watch again; should clear capability from scope + mockScope.$watch.calls[0].args[1](); + expect(mockScope.testCapability).toBeUndefined(); + }); }); } ); \ No newline at end of file From a18a7c99608a517503f632b84cbabc3005d52e5f Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 3 Jun 2015 07:49:11 -0700 Subject: [PATCH 4/4] [Layout] Fix comment Change inaccurate comment based on feedback from code review for WTD-1182. --- platform/features/layout/src/LayoutController.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/platform/features/layout/src/LayoutController.js b/platform/features/layout/src/LayoutController.js index f3d44110b5..9212f900b3 100644 --- a/platform/features/layout/src/LayoutController.js +++ b/platform/features/layout/src/LayoutController.js @@ -69,7 +69,8 @@ define( function lookupPanels(ids) { var configuration = $scope.configuration || {}; - // Ensure ids is array-like + // ids is read from model.composition and may be undefined; + // fall back to an array if that occurs ids = ids || []; // Pull panel positions from configuration