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

fix(pagination): paging buttons stay focused when page is changed #3486 #3488

Closed
wants to merge 3 commits into from
Closed

fix(pagination): paging buttons stay focused when page is changed #3486 #3488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6eb3eea
Passed the ngClick $event to selectPage in order blur the button so i…
benthepoet Apr 4, 2015
eb90032
fix(pagination): pager tests to verify buttons are blurred on click
benthepoet Apr 5, 2015
9100343
fix(pagination): pagination tests for blur on click
benthepoet Apr 5, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/pagination/pagination.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ angular.module('ui.bootstrap.pagination', [])
$scope.page = parseInt(ngModelCtrl.$viewValue, 10) || 1;
};

$scope.selectPage = function(page) {
$scope.selectPage = function(page, evt) {
if ( $scope.page !== page && page > 0 && page <= $scope.totalPages) {
if (evt && evt.target) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this ever be called without an event object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, below is the only spot where it gets called without an event.

https://github.com/benthepoet/bootstrap/blob/pagination-fix/src/pagination/pagination.js#L59-L68

evt.target.blur();
}
ngModelCtrl.$setViewValue(page);
ngModelCtrl.$render();
}
Expand Down
35 changes: 33 additions & 2 deletions src/pagination/test/pager.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
describe('pager directive', function () {
var $compile, $rootScope, element;
var $compile, $rootScope, $document, element;
beforeEach(module('ui.bootstrap.pagination'));
beforeEach(module('template/pagination/pager.html'));
beforeEach(inject(function(_$compile_, _$rootScope_) {
beforeEach(inject(function(_$compile_, _$rootScope_, _$document_) {
$compile = _$compile_;
$rootScope = _$rootScope_;
$rootScope.total = 47; // 5 pages
$rootScope.currentPage = 3;
$document = _$document_;
element = $compile('<pager total-items="total" ng-model="currentPage"></pager>')($rootScope);
$rootScope.$digest();
}));
Expand All @@ -22,6 +23,10 @@ describe('pager directive', function () {
function clickPaginationEl(index) {
getPaginationEl(index).find('a').click();
}

function getPaginationLinkEl(elem, index) {
return elem.find('li').eq(index).find('a');
}

function updateCurrentPage(value) {
$rootScope.currentPage = value;
Expand Down Expand Up @@ -96,6 +101,32 @@ describe('pager directive', function () {
expect(getPaginationEl(-1).text()).toBe('Next »');
});

it('should blur the "next" link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, -1);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});

it('should blur the "prev" link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, -1);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});

describe('`items-per-page`', function () {
beforeEach(function() {
$rootScope.perpage = 5;
Expand Down
87 changes: 85 additions & 2 deletions src/pagination/test/pagination.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
describe('pagination directive', function () {
var $compile, $rootScope, element;
var $compile, $rootScope, $document, element;
beforeEach(module('ui.bootstrap.pagination'));
beforeEach(module('template/pagination/pagination.html'));
beforeEach(inject(function(_$compile_, _$rootScope_) {
beforeEach(inject(function(_$compile_, _$rootScope_, _$document_) {
$compile = _$compile_;
$rootScope = _$rootScope_;
$rootScope.total = 47; // 5 pages
$rootScope.currentPage = 3;
$document = _$document_;
element = $compile('<pagination total-items="total" ng-model="currentPage"></pagination>')($rootScope);
$rootScope.$digest();
}));
Expand All @@ -22,6 +23,10 @@ describe('pagination directive', function () {
function clickPaginationEl(index) {
getPaginationEl(index).find('a').click();
}

function getPaginationLinkEl(elem, index) {
return elem.find('li').eq(index).find('a');
}

function updateCurrentPage(value) {
$rootScope.currentPage = value;
Expand Down Expand Up @@ -122,6 +127,45 @@ describe('pagination directive', function () {
expect($rootScope.currentPage).toBe(1);
});

it('should blur a page link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, 2);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});

it('should blur the "next" link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, -1);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});

it('should blur the "prev" link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, 0);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});

describe('`items-per-page`', function () {
beforeEach(function() {
$rootScope.perpage = 5;
Expand Down Expand Up @@ -259,6 +303,19 @@ describe('pagination directive', function () {
expect(getPaginationEl(0).text()).toBe('Previous');
expect(getPaginationEl(-1).text()).toBe('Next');
});

it('should blur page link when visible range changes', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, 4);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});
});

describe('with `max-size` option & no `rotate`', function () {
Expand Down Expand Up @@ -415,6 +472,32 @@ describe('pagination directive', function () {
expect(getPaginationEl(1).text()).toBe('<<');
expect(getPaginationEl(-2).text()).toBe('>>');
});

it('should blur the "first" link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, 0);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});

it('should blur the "last" link after it has been clicked', function () {
$document.find('body').append(element);
var linkEl = getPaginationLinkEl(element, -1);

linkEl.focus();
expect(linkEl).toHaveFocus();

linkEl.click();
expect(linkEl).not.toHaveFocus();

element.remove();
});
});

describe('pagination directive with just number links', function () {
Expand Down
4 changes: 2 additions & 2 deletions template/pagination/pager.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ul class="pager">
<li ng-class="{disabled: noPrevious(), previous: align}"><a href ng-click="selectPage(page - 1)">{{getText('previous')}}</a></li>
<li ng-class="{disabled: noNext(), next: align}"><a href ng-click="selectPage(page + 1)">{{getText('next')}}</a></li>
<li ng-class="{disabled: noPrevious(), previous: align}"><a href ng-click="selectPage(page - 1, $event)">{{getText('previous')}}</a></li>
<li ng-class="{disabled: noNext(), next: align}"><a href ng-click="selectPage(page + 1, $event)">{{getText('next')}}</a></li>
</ul>
10 changes: 5 additions & 5 deletions template/pagination/pagination.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<ul class="pagination">
<li ng-if="boundaryLinks" ng-class="{disabled: noPrevious()}"><a href ng-click="selectPage(1)">{{getText('first')}}</a></li>
<li ng-if="directionLinks" ng-class="{disabled: noPrevious()}"><a href ng-click="selectPage(page - 1)">{{getText('previous')}}</a></li>
<li ng-repeat="page in pages track by $index" ng-class="{active: page.active}"><a href ng-click="selectPage(page.number)">{{page.text}}</a></li>
<li ng-if="directionLinks" ng-class="{disabled: noNext()}"><a href ng-click="selectPage(page + 1)">{{getText('next')}}</a></li>
<li ng-if="boundaryLinks" ng-class="{disabled: noNext()}"><a href ng-click="selectPage(totalPages)">{{getText('last')}}</a></li>
<li ng-if="boundaryLinks" ng-class="{disabled: noPrevious()}"><a href ng-click="selectPage(1, $event)">{{getText('first')}}</a></li>
<li ng-if="directionLinks" ng-class="{disabled: noPrevious()}"><a href ng-click="selectPage(page - 1, $event)">{{getText('previous')}}</a></li>
<li ng-repeat="page in pages track by $index" ng-class="{active: page.active}"><a href ng-click="selectPage(page.number, $event)">{{page.text}}</a></li>
<li ng-if="directionLinks" ng-class="{disabled: noNext()}"><a href ng-click="selectPage(page + 1, $event)">{{getText('next')}}</a></li>
<li ng-if="boundaryLinks" ng-class="{disabled: noNext()}"><a href ng-click="selectPage(totalPages, $event)">{{getText('last')}}</a></li>
</ul>