Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

fix(tabs): keep stable tab order (when filtering ng-repeat) #153

Closed
wants to merge 6 commits into from
82 changes: 76 additions & 6 deletions src/tabs/tabs.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
angular.module('ui.bootstrap.tabs', [])
.controller('TabsController', ['$scope', '$element', function($scope, $element) {
var paneRecords = [];
var panes = $scope.panes = [];

this.select = $scope.select = function selectPane(pane) {
Expand All @@ -9,17 +10,87 @@ angular.module('ui.bootstrap.tabs', [])
pane.selected = true;
};

this.addPane = function addPane(pane) {
this.sortPaneRecords = function sortPaneRecords() {
var indexing = [];

// for pane record return array of indexes for sorting
// the function walks up the DOM from passed pane up to <tabs> element
// and collects order index of element on each DOM level
var computeIndexing = function computeIndexing(record) {
if (!record.element) {
return [];
}
var node = record.element.get(0);
var res = [];
var rawTabsRootElement = $element.get(0);
while (node) {
var index = Array.prototype.indexOf.call(node.parentNode.childNodes, node);
res.push(index);
node = node.parentNode;
if (node==rawTabsRootElement) {
break;
}
}
return res.reverse(); // indices go in order from [tabs's 1st child, 2nd child, ..., pane]
};

// pre-compute indexing structure for each record
angular.forEach(paneRecords, function(record) {
record.indexing = computeIndexing(record);
});

// indexing structure gives us effectively global order index of an element in the DOM tree
// this function orders records in the order of their appearance in the DOM (top-bottom)
var byIndexing = function byIndexing(a, b) {
var i = 0;
a = a.indexing;
b = b.indexing;
while (true) {
// "b goes first" or "b is parent of a"
if (b[i]<a[i] || (b[i]===undefined && a[i]!==undefined)) {
return true;
}
// "a goes first" or "a is parent of b"
if (a[i]<b[i] || (a[i]===undefined && b[i]!==undefined)) {
return false;
}
// this should never happen unless we have inconsistent data
if (a[i]===undefined && b[i]===undefined) {
break; // prevent infinite loop
}
i++;
}
return false;
};

paneRecords.sort(byIndexing);
};

this.updatePanes = function updatePanes() {
panes.splice(0, panes.length);
angular.forEach(paneRecords, function(record) {
panes.push(record.pane);
});
};

this.addPane = function addPane(pane, element) {
if (!panes.length) {
$scope.select(pane);
}
panes.push(pane);
paneRecords.push({
pane: pane,
element: element
});
// we need to sort panes array to match the DOM order, see https://github.com/angular-ui/bootstrap/pull/153
this.sortPaneRecords();
this.updatePanes();
};

this.removePane = function removePane(pane) {
this.removePane = function removePane(pane) {
var index = panes.indexOf(pane);
panes.splice(index, 1);
//Select a new pane if removed pane was selected
paneRecords.splice(index, 1);
//Select a new pane if removed pane was selected
if (pane.selected && panes.length > 0) {
$scope.select(panes[index < panes.length ? index : index-1]);
}
Expand Down Expand Up @@ -63,8 +134,7 @@ angular.module('ui.bootstrap.tabs', [])
setSelected(scope.$parent, selected);
}
});

tabsCtrl.addPane(scope);
tabsCtrl.addPane(scope, element);
scope.$on('$destroy', function() {
tabsCtrl.removePane(scope);
});
Expand Down
124 changes: 123 additions & 1 deletion src/tabs/test/tabsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ describe('remote selection', function() {
expect(titles.eq(1)).toHaveClass('active');

titles.eq(0).find('a').click();

expect(scope.panes[1].active).toBe(false);
});

Expand Down Expand Up @@ -255,3 +255,125 @@ describe('remove tabs', function() {

});

describe('keep stable tab order', function() {

// load the tabs code
beforeEach(module('ui.bootstrap.tabs'));

// load the templates
beforeEach(module('template/tabs/tabs.html', 'template/tabs/pane.html'));

it('should reintroduce pane to proper position', inject(function($compile, $rootScope) {
// we might move this tpl into an html file as well...
var elm = angular.element(
'<div>' +
'<tabs>' +
'<pane ng-repeat="pane in panes | filter:paneIsAvailable" active="pane.active" heading="{{pane.title}}">' +
'some content' +
'</pane>' +
'</tabs>' +
'</div>'
);
var scope = $rootScope;
scope.panes = [
{ title:"Title 1", available:true },
{ title:"Title 2", available:true },
{ title:"Title 3", available:true }
];

scope.paneIsAvailable = function(pane) {
return pane.available;
};

$compile(elm)(scope);
scope.$digest();

function titles() {
return elm.find('ul.nav-tabs li');
}

expect(titles().length).toBe(3);
scope.$apply('panes[1].available=false');
scope.$digest();
expect(titles().length).toBe(2);
scope.$apply('panes[1].available=true');
scope.$digest();
expect(titles().length).toBe(3);
expect(titles().eq(0).text().trim()).toBe("Title 1");
expect(titles().eq(1).text().trim()).toBe("Title 2");
expect(titles().eq(2).text().trim()).toBe("Title 3");
}));

it('should reintroduce pane to proper position even with garbage', inject(function($compile, $rootScope) {
// we might move this tpl into an html file as well...
var elm = angular.element(
'<div>' +
'<tabs>' +
' <!-- a comment -->' +
' <div>div that makes troubles</div>' +
'<pane heading="first">First Static</pane>' +
' <div>another div that may do evil</div>' +
'<pane ng-repeat="pane in panes | filter:paneIsAvailable" active="pane.active" heading="{{pane.title}}">some content</pane>' +
' <!-- another comment -->' +
'<pane heading="mid">Mid Static</pane>' +
' a text node' +
' <!-- another comment -->' +
' <span>yet another span that may do evil</span>' +
'<pane ng-repeat="pane in panes | filter:paneIsAvailable" active="pane.active" heading="Second {{pane.title}}">some content</pane>' +
' a text node' +
' <span>yet another span that may do evil</span>' +
' <!-- another comment -->' +
'<pane heading="last">Last Static</pane>' +
' a text node' +
' <span>yet another span that may do evil</span>' +
' <!-- another comment -->' +
'</tabs>' +
'</div>'
);
var scope = $rootScope;
scope.panes = [
{ title:"Title 1", available:true },
{ title:"Title 2", available:true },
{ title:"Title 3", available:true }
];

scope.paneIsAvailable = function(pane) {
return pane.available;
};

function titles() {
return elm.find('ul.nav-tabs li');
}

$compile(elm)(scope);
scope.$digest();

expect(titles().length).toBe(9);
scope.$apply('panes[1].available=false');
scope.$digest();
expect(titles().length).toBe(7);
scope.$apply('panes[0].available=false');
scope.$digest();
expect(titles().length).toBe(5);
scope.$apply('panes[2].available=false');
scope.$digest();
expect(titles().length).toBe(3);
scope.$apply('panes[0].available=true');
scope.$digest();
expect(titles().length).toBe(5);
scope.$apply('panes[1].available=true');
scope.$apply('panes[2].available=true');
scope.$digest();
expect(titles().length).toBe(9);
expect(titles().eq(0).text().trim()).toBe("first");
expect(titles().eq(1).text().trim()).toBe("Title 1");
expect(titles().eq(2).text().trim()).toBe("Title 2");
expect(titles().eq(3).text().trim()).toBe("Title 3");
expect(titles().eq(4).text().trim()).toBe("mid");
expect(titles().eq(5).text().trim()).toBe("Second Title 1");
expect(titles().eq(6).text().trim()).toBe("Second Title 2");
expect(titles().eq(7).text().trim()).toBe("Second Title 3");
expect(titles().eq(8).text().trim()).toBe("last");
}));

});