Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Commit

Permalink
fix(tabs): delays disconnect until after the digest is finished
Browse files Browse the repository at this point in the history
Closes #1048.
  • Loading branch information
robertmesserle committed Jan 7, 2015
1 parent 993fa2b commit 78ba497
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
10 changes: 7 additions & 3 deletions src/components/tabs/js/tabItemController.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
angular.module('material.components.tabs')
.controller('$mdTab', TabItemController);

function TabItemController($scope, $element, $attrs, $compile, $animate, $mdUtil, $parse) {
function TabItemController($scope, $element, $attrs, $compile, $animate, $mdUtil, $parse, $timeout) {
var self = this;

// Properties
Expand All @@ -29,14 +29,18 @@ function TabItemController($scope, $element, $attrs, $compile, $animate, $mdUtil
* Add the tab's content to the DOM container area in the tabs,
* @param contentArea the contentArea to add the content of the tab to
*/
function onAdd(contentArea) {
function onAdd(contentArea, shouldDisconnectScope) {
if (self.content.length) {
self.contentContainer.append(self.content);
self.contentScope = $scope.$parent.$new();
contentArea.append(self.contentContainer);

$compile(self.contentContainer)(self.contentScope);
$mdUtil.disconnectScope(self.contentScope);
if (shouldDisconnectScope === true) {
$timeout(function () {
$mdUtil.disconnectScope(self.contentScope);
}, 0, false);

This comment has been minimized.

Copy link
@mxdubois

mxdubois Feb 26, 2015

This causes a problem for me where the scope is disconnected after it is reconnected by onSelect during the initial load. The bug results in ng-model bindings in the initially selected tab not working until the user switches to another tab and switches back.

In what case is the $timeout necessary? If we need to keep the $timeout, we should save the promise returned by $timeout and cancel it in onSelect where we call reconnectScope. If that sounds good, I can submit a PR.

This comment has been minimized.

Copy link
@robertmesserle

robertmesserle Feb 26, 2015

Author Contributor

That sounds reasonable. I'm going to open a ticket for this; however, there may be changes coming for Tabs soon, so it may become a non-issue.

}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/components/tabs/js/tabsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ function MdTabsController($scope, $element, $mdUtil, $$rAF) {
// Returns a method to remove the tab from the list.
function add(tab, index) {
tabsList.add(tab, index);
tab.onAdd(self.contentArea);

// Select the new tab if we don't have a selectedIndex, or if the
// selectedIndex we've been waiting for is this tab
if ($scope.selectedIndex === -1 || !angular.isNumber($scope.selectedIndex) ||
$scope.selectedIndex === self.indexOf(tab)) {
tab.onAdd(self.contentArea, false);
self.select(tab);
} else {
tab.onAdd(self.contentArea, true);
}

$scope.$broadcast('$mdTabsChanged');
Expand Down
5 changes: 3 additions & 2 deletions src/components/tabs/tabs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,21 +116,22 @@ describe('<md-tabs>', function() {
expect(tabItems.eq(0)).toBeActiveTab();
}));

it('the active tab\'s content should always be connected', function() {
it('the active tab\'s content should always be connected', inject(function($timeout) {
var tabs = setup('<md-tabs>' +
'<md-tab label="label1!">content1!</md-tab>' +
'<md-tab label="label2!">content2!</md-tab>' +
'</md-tabs>');
var tabItems = tabs.find('md-tab');
var contents = angular.element(tabs[0].querySelectorAll('.md-tab-content'));

$timeout.flush();
expect(contents.eq(0).scope().$$disconnected).toBeFalsy();
expect(contents.eq(1).scope().$$disconnected).toBeTruthy();

tabItems.eq(1).triggerHandler('click');
expect(contents.eq(0).scope().$$disconnected).toBeTruthy();
expect(contents.eq(1).scope().$$disconnected).toBeFalsy();
});
}));

it('should bind to selected', function() {
var tabs = setup('<md-tabs md-selected="current">' +
Expand Down

0 comments on commit 78ba497

Please sign in to comment.