Skip to content

Commit d8f1196

Browse files
author
Alexandre Stanislawski
committed
fix(pagination): fix #668 edge case
Also code simplification
1 parent 91e91f7 commit d8f1196

File tree

3 files changed

+179
-9
lines changed

3 files changed

+179
-9
lines changed

dev/debug.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
../css/debug.css
1+
../src/css/debug.css

src/components/Pagination/Paginator.js

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,31 @@ class Paginator {
88
}
99

1010
pages() {
11-
let current = this.currentPage;
12-
let padding = this.padding;
13-
let paddingLeft = Math.min(this.calculatePaddingLeft(current, padding, this.total), this.total);
14-
let paddingRight = Math.max(Math.min(2 * padding + 1, this.total) - paddingLeft, 1);
15-
let first = Math.max(current - paddingLeft, 0);
16-
let last = current + paddingRight;
11+
const {total, currentPage, padding} = this;
12+
13+
const totalDisplayedPages = this.nbPagesDisplayed(padding, total);
14+
if (totalDisplayedPages === total) return range(0, total);
15+
16+
const paddingLeft = this.calculatePaddingLeft(currentPage, padding, total, totalDisplayedPages);
17+
const paddingRight = totalDisplayedPages - paddingLeft;
18+
19+
const first = currentPage - paddingLeft;
20+
const last = currentPage + paddingRight;
21+
1722
return range(first, last);
1823
}
1924

20-
calculatePaddingLeft(current, padding, total) {
25+
nbPagesDisplayed(padding, total) {
26+
return Math.min(2 * padding + 1, total);
27+
}
28+
29+
calculatePaddingLeft(current, padding, total, totalDisplayedPages) {
2130
if (current <= padding) {
2231
return current;
2332
}
2433

2534
if (current >= (total - padding)) {
26-
return 2 * padding + 1 - (total - current);
35+
return totalDisplayedPages - (total - current);
2736
}
2837

2938
return padding;
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/* eslint-env mocha */
2+
3+
import expect from 'expect';
4+
5+
import Paginator from '../Paginator';
6+
7+
describe('paginator: simple cases', () => {
8+
context('on the first page', () => {
9+
const pager = new Paginator({
10+
currentPage: 0,
11+
total: 10,
12+
padding: 2
13+
});
14+
15+
it('should return the pages', () => {
16+
const pages = pager.pages();
17+
expect(pages.length).toBe(5);
18+
expect(pages).toEqual([0, 1, 2, 3, 4]);
19+
});
20+
21+
it('should be the first page', () => {
22+
expect(pager.isFirstPage()).toBe(true);
23+
});
24+
25+
it('should not be the last page', () => {
26+
expect(pager.isLastPage()).toBe(false);
27+
});
28+
});
29+
30+
context('on 3rd page', () => {
31+
const pager = new Paginator({
32+
currentPage: 2,
33+
total: 10,
34+
padding: 2
35+
});
36+
37+
it('should return the pages', () => {
38+
const pages = pager.pages();
39+
expect(pages.length).toBe(5);
40+
expect(pages).toEqual([0, 1, 2, 3, 4]);
41+
});
42+
43+
it('should not be the first page', () => {
44+
expect(pager.isFirstPage()).toBe(false);
45+
});
46+
47+
it('should not be the last page', () => {
48+
expect(pager.isLastPage()).toBe(false);
49+
});
50+
});
51+
52+
context('on 5th page', () => {
53+
const pager = new Paginator({
54+
currentPage: 5,
55+
total: 10,
56+
padding: 2
57+
});
58+
59+
it('should return the pages', () => {
60+
const pages = pager.pages();
61+
expect(pages.length).toBe(5);
62+
expect(pages).toEqual([3, 4, 5, 6, 7]);
63+
});
64+
65+
it('should not be the first page', () => {
66+
expect(pager.isFirstPage()).toBe(false);
67+
});
68+
69+
it('should not be the last page', () => {
70+
expect(pager.isLastPage()).toBe(false);
71+
});
72+
});
73+
74+
context('on the page before the last', () => {
75+
const pager = new Paginator({
76+
currentPage: 8,
77+
total: 10,
78+
padding: 2
79+
});
80+
81+
it('should return the pages', () => {
82+
const pages = pager.pages();
83+
expect(pages.length).toBe(5);
84+
expect(pages).toEqual([5, 6, 7, 8, 9]);
85+
});
86+
87+
it('should not be the first page', () => {
88+
expect(pager.isFirstPage()).toBe(false);
89+
});
90+
91+
it('should not be the last page', () => {
92+
expect(pager.isLastPage()).toBe(false);
93+
});
94+
});
95+
96+
context('on last page', () => {
97+
const pager = new Paginator({
98+
currentPage: 9,
99+
total: 10,
100+
padding: 2
101+
});
102+
103+
it('should return the pages', () => {
104+
const pages = pager.pages();
105+
expect(pages.length).toBe(5);
106+
expect(pages).toEqual([5, 6, 7, 8, 9]);
107+
});
108+
109+
it('should not be the first page', () => {
110+
expect(pager.isFirstPage()).toBe(false);
111+
});
112+
113+
it('should not be the last page', () => {
114+
expect(pager.isLastPage()).toBe(true);
115+
});
116+
});
117+
});
118+
119+
describe('paginator: number of pages is less than 2*padding+1', () => {
120+
const pager = new Paginator({
121+
currentPage: 0,
122+
total: 1,
123+
padding: 2
124+
});
125+
126+
it('should return the pages', () => {
127+
const pages = pager.pages();
128+
expect(pages.length).toBe(1);
129+
expect(pages).toEqual([0]);
130+
});
131+
132+
it('should be the first page', () => {
133+
expect(pager.isFirstPage()).toBe(true);
134+
});
135+
136+
it('should not be the last page', () => {
137+
expect(pager.isLastPage()).toBe(true);
138+
});
139+
});
140+
141+
describe('paginator: bug #668', () => {
142+
const pager = new Paginator({
143+
currentPage: 4,
144+
total: 6,
145+
padding: 3
146+
});
147+
148+
it('should return the pages', () => {
149+
const pages = pager.pages();
150+
expect(pages.length).toBe(6);
151+
expect(pages).toEqual([0, 1, 2, 3, 4, 5]);
152+
});
153+
154+
it('should be the first page', () => {
155+
expect(pager.isFirstPage()).toBe(false);
156+
});
157+
158+
it('should not be the last page', () => {
159+
expect(pager.isLastPage()).toBe(false);
160+
});
161+
});

0 commit comments

Comments
 (0)