Skip to content

Commit 2ede317

Browse files
committed
fix(pagination): Fix double BEM classes on elements
Fixes #500 BREAKING CHANGE: Removes all `__disabled`, `__first`, `__last`, `__next`, `__previous`, `__active` and `__page` classes added on the links in the pagination. It only ads them to the parent `li`. Links instead now have a `.ais-pagination--link` class Previously, the same CSS classes where added to both the `item` (`li`) and the link inside it. I've split them in `--item` and `--link`. I've also made the various active/first/disabled/etc modifiers as actual `__modifier` classes. I've updated the tests, the CSS skeleton, the examples and the docs accordingly.
1 parent 1bbd569 commit 2ede317

File tree

9 files changed

+82
-69
lines changed

9 files changed

+82
-69
lines changed

components/Pagination/Pagination.js

+14-9
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,25 @@ class Pagination extends React.Component {
2323
this.props.setCurrentPage(pageNumber);
2424
}
2525

26-
pageLink({label, ariaLabel, pageNumber, className = null, isDisabled = false, isActive = false, createURL}) {
26+
pageLink({label, ariaLabel, pageNumber, additionalClassName = null, isDisabled = false, isActive = false, createURL}) {
2727
let handleClick = this.handleClick.bind(this, pageNumber);
2828

29+
let cssClasses = {
30+
item: cx(this.props.cssClasses.item, additionalClassName),
31+
link: cx(this.props.cssClasses.link)
32+
};
2933
if (isDisabled) {
30-
className = cx(this.props.cssClasses.disabled, className);
34+
cssClasses.item = cx(cssClasses.item, this.props.cssClasses.disabled);
3135
} else if (isActive) {
32-
className = cx(this.props.cssClasses.active, className);
36+
cssClasses.item = cx(cssClasses.item, this.props.cssClasses.active);
3337
}
3438

3539
let url = createURL && !isDisabled ? createURL(pageNumber) : '#';
3640

3741
return (
3842
<PaginationLink
3943
ariaLabel={ariaLabel}
40-
className={className}
44+
cssClasses={cssClasses}
4145
handleClick={handleClick}
4246
key={label}
4347
label={label}
@@ -49,7 +53,7 @@ class Pagination extends React.Component {
4953
previousPageLink(pager, createURL) {
5054
return this.pageLink({
5155
ariaLabel: 'Previous',
52-
className: this.props.cssClasses.previous,
56+
additionalClassName: this.props.cssClasses.previous,
5357
isDisabled: pager.isFirstPage(),
5458
label: this.props.labels.previous,
5559
pageNumber: pager.currentPage - 1,
@@ -60,7 +64,7 @@ class Pagination extends React.Component {
6064
nextPageLink(pager, createURL) {
6165
return this.pageLink({
6266
ariaLabel: 'Next',
63-
className: this.props.cssClasses.next,
67+
additionalClassName: this.props.cssClasses.next,
6468
isDisabled: pager.isLastPage(),
6569
label: this.props.labels.next,
6670
pageNumber: pager.currentPage + 1,
@@ -71,7 +75,7 @@ class Pagination extends React.Component {
7175
firstPageLink(pager, createURL) {
7276
return this.pageLink({
7377
ariaLabel: 'First',
74-
className: this.props.cssClasses.first,
78+
additionalClassName: this.props.cssClasses.first,
7579
isDisabled: pager.isFirstPage(),
7680
label: this.props.labels.first,
7781
pageNumber: 0,
@@ -82,7 +86,7 @@ class Pagination extends React.Component {
8286
lastPageLink(pager, createURL) {
8387
return this.pageLink({
8488
ariaLabel: 'Last',
85-
className: this.props.cssClasses.last,
89+
additionalClassName: this.props.cssClasses.last,
8690
isDisabled: pager.isLastPage(),
8791
label: this.props.labels.last,
8892
pageNumber: pager.total - 1,
@@ -98,7 +102,7 @@ class Pagination extends React.Component {
98102

99103
pages.push(this.pageLink({
100104
ariaLabel: pageNumber + 1,
101-
className: this.props.cssClasses.page,
105+
additionalClassName: this.props.cssClasses.page,
102106
isActive: isActive,
103107
label: pageNumber + 1,
104108
pageNumber: pageNumber,
@@ -135,6 +139,7 @@ Pagination.propTypes = {
135139
cssClasses: React.PropTypes.shape({
136140
root: React.PropTypes.string,
137141
item: React.PropTypes.string,
142+
link: React.PropTypes.string,
138143
page: React.PropTypes.string,
139144
previous: React.PropTypes.string,
140145
next: React.PropTypes.string,

components/Pagination/PaginationLink.js

+7-4
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ let React = require('react');
22

33
class PaginationLink extends React.Component {
44
render() {
5-
let {className, label, ariaLabel, handleClick, url} = this.props;
5+
let {cssClasses, label, ariaLabel, handleClick, url} = this.props;
66

77
return (
8-
<li className={className}>
8+
<li className={cssClasses.item}>
99
<a
1010
ariaLabel={ariaLabel}
11-
className={className}
11+
className={cssClasses.link}
1212
dangerouslySetInnerHTML={{__html: label}}
1313
href={url}
1414
onClick={handleClick}
@@ -23,7 +23,10 @@ PaginationLink.propTypes = {
2323
React.PropTypes.string,
2424
React.PropTypes.number
2525
]).isRequired,
26-
className: React.PropTypes.string,
26+
cssClasses: React.PropTypes.shape({
27+
item: React.PropTypes.string,
28+
link: React.PropTypes.string
29+
}),
2730
handleClick: React.PropTypes.func.isRequired,
2831
label: React.PropTypes.oneOfType([
2932
React.PropTypes.string,

components/Pagination/__tests__/Pagination-test.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ describe('Pagination', () => {
4848
it('should flag the current page as active', () => {
4949
let out = render({currentPage: 0});
5050

51-
expect(out.props.children[2][0].props.className).toBe('active page');
52-
expect(out.props.children[2][1].props.className).toBe('page');
51+
expect(out.props.children[2][0].props.cssClasses.item).toBe('item page active');
52+
expect(out.props.children[2][1].props.cssClasses.item).toBe('item page');
5353
});
5454

5555
it('should disable the first page if already on it', () => {
5656
let out = render({currentPage: 0, showFirstLast: true});
5757

58-
expect(out.props.children[0].props.className).toBe('disabled first');
58+
expect(out.props.children[0].props.cssClasses.item).toBe('item first disabled');
5959
});
6060

6161
it('should build the associated URL', () => {
@@ -68,7 +68,7 @@ describe('Pagination', () => {
6868
expect(out).toEqualJSX(
6969
<PaginationLink
7070
ariaLabel={undefined}
71-
className={null}
71+
cssClasses={{item: '', link: ''}}
7272
handleClick={() => {}}
7373
key="test"
7474
label="test"
@@ -88,7 +88,7 @@ describe('Pagination', () => {
8888
expect(out).toEqualJSX(
8989
<PaginationLink
9090
ariaLabel={undefined}
91-
className=""
91+
cssClasses={{item: '', link: ''}}
9292
handleClick={() => {}}
9393
key="test"
9494
label="test"
@@ -97,10 +97,10 @@ describe('Pagination', () => {
9797
expect(createURL.called).toBe(false, 'createURL should not be called');
9898
});
9999

100-
it('should disable last first page if already on it', () => {
100+
it('should disable last page if already on it', () => {
101101
let out = render({currentPage: 19, showFirstLast: true});
102102

103-
expect(out.props.children[4].props.className).toBe('disabled last');
103+
expect(out.props.children[4].props.cssClasses.item).toBe('item last disabled');
104104
});
105105

106106
it('should handle special clicks', () => {

css/default/_pagination.scss

+18-15
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,26 @@
88
/* disabled pagination item */
99
visibility: hidden;
1010
}
11-
}
12-
@include element(item-first) {
13-
/* first pagination item */
14-
}
15-
@include element(item-previous) {
16-
/* previous pagination item */
17-
}
18-
@include element(item-page) {
19-
/* page */
2011
@include modifier(active) {
21-
/* active page */
12+
/* active pagination item */
13+
}
14+
@include modifier(first) {
15+
/* first pagination item */
16+
}
17+
@include modifier(previous) {
18+
/* previous pagination item */
19+
}
20+
@include modifier(page) {
21+
/* page pagination item */
22+
}
23+
@include modifier(next) {
24+
/* next pagination item */
25+
}
26+
@include modifier(last) {
27+
/* last pagination item */
2228
}
2329
}
24-
@include element(item-next) {
25-
/* next pagination item */
26-
}
27-
@include element(item-last) {
28-
/* last pagination item */
30+
@include element(link) {
31+
/* pagination link */
2932
}
3033
}

docs/_includes/widget-jsdoc/pagination.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
| <span class='attr-optional'>`options.cssClasses`</span> | CSS classes to be added |
55
| <span class='attr-optional'>`options.cssClasses.root`</span> | CSS classes added to the parent <ul> |
66
| <span class='attr-optional'>`options.cssClasses.item`</span> | CSS classes added to each <li> |
7+
| <span class='attr-optional'>`options.cssClasses.link`</span> | CSS classes added to each link |
78
| <span class='attr-optional'>`options.cssClasses.page`</span> | CSS classes added to page <li> |
89
| <span class='attr-optional'>`options.cssClasses.previous`</span> | CSS classes added to the previous <li> |
910
| <span class='attr-optional'>`options.cssClasses.next`</span> | CSS classes added to the next <li> |

docs/examples/airbnb/scss/_pagination.scss

+8-10
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,13 @@
33

44
@include block(pagination) {
55
@include element(item) {
6-
@include modifier(disabled) {
7-
visibility: hidden;
8-
}
6+
@include modifier(disabled)
7+
@include modifier(active)
8+
@include modifier(first)
9+
@include modifier(previous)
10+
@include modifier(page)
11+
@include modifier(next)
12+
@include modifier(last)
913
}
10-
@include element(item-first);
11-
@include element(item-previous);
12-
@include element(item-page) {
13-
@include modifier(active);
14-
}
15-
@include element(item-next);
16-
@include element(item-last);
14+
@include element(link)
1715
}

docs/examples/youtube/scss/_pagination.scss

+9-10
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@
33

44
@include block(pagination) {
55
@include element(item) {
6-
@include modifier(disabled) {
7-
visibility: hidden;
8-
}
6+
@include modifier(disabled)
7+
@include modifier(active)
8+
@include modifier(first)
9+
@include modifier(previous)
10+
@include modifier(page)
11+
@include modifier(next)
12+
@include modifier(last)
913
}
10-
@include element(item-first);
11-
@include element(item-previous);
12-
@include element(item-page) {
13-
@include modifier(active);
14-
}
15-
@include element(item-next);
16-
@include element(item-last);
14+
@include element(link)
1715
}
16+

widgets/pagination/__tests__/pagination-test.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ describe('pagination()', () => {
3030
cssClasses = {
3131
root: 'root',
3232
item: 'item',
33+
link: 'link',
3334
page: 'page',
3435
previous: 'previous',
3536
next: 'next',
@@ -107,13 +108,14 @@ describe('pagination()', () => {
107108
cssClasses: {
108109
root: 'ais-pagination root',
109110
item: 'ais-pagination--item item',
110-
page: 'ais-pagination--item ais-pagination--item-page page',
111-
previous: 'ais-pagination--item ais-pagination--item-previous previous',
112-
next: 'ais-pagination--item ais-pagination--item-next next',
113-
first: 'ais-pagination--item ais-pagination--item-first first',
114-
last: 'ais-pagination--item ais-pagination--item-last last',
115-
active: 'ais-pagination--item ais-pagination--item-page ais-pagination--item-page__active active',
116-
disabled: 'ais-pagination--item ais-pagination--item__disabled disabled'
111+
link: 'ais-pagination--link link',
112+
page: 'ais-pagination--item__page page',
113+
previous: 'ais-pagination--item__previous previous',
114+
next: 'ais-pagination--item__next next',
115+
first: 'ais-pagination--item__first first',
116+
last: 'ais-pagination--item__last last',
117+
active: 'ais-pagination--item__active active',
118+
disabled: 'ais-pagination--item__disabled disabled'
117119
},
118120
currentPage: 0,
119121
shouldAutoHideContainer: false,

widgets/pagination/pagination.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ let defaultLabels = {
2020
* @param {Object} [options.cssClasses] CSS classes to be added
2121
* @param {string} [options.cssClasses.root] CSS classes added to the parent <ul>
2222
* @param {string} [options.cssClasses.item] CSS classes added to each <li>
23+
* @param {string} [options.cssClasses.link] CSS classes added to each link
2324
* @param {string} [options.cssClasses.page] CSS classes added to page <li>
2425
* @param {string} [options.cssClasses.previous] CSS classes added to the previous <li>
2526
* @param {string} [options.cssClasses.next] CSS classes added to the next <li>
@@ -84,13 +85,14 @@ function pagination({
8485
let cssClasses = {
8586
root: cx(bem(null), userCssClasses.root),
8687
item: cx(bem('item'), userCssClasses.item),
87-
page: cx(bem('item'), bem('item-page'), userCssClasses.page),
88-
previous: cx(bem('item'), bem('item-previous'), userCssClasses.previous),
89-
next: cx(bem('item'), bem('item-next'), userCssClasses.next),
90-
first: cx(bem('item'), bem('item-first'), userCssClasses.first),
91-
last: cx(bem('item'), bem('item-last'), userCssClasses.last),
92-
active: cx(bem('item'), bem('item-page'), bem('item-page', 'active'), userCssClasses.active),
93-
disabled: cx(bem('item'), bem('item', 'disabled'), userCssClasses.disabled)
88+
link: cx(bem('link'), userCssClasses.link),
89+
page: cx(bem('item', 'page'), userCssClasses.page),
90+
previous: cx(bem('item', 'previous'), userCssClasses.previous),
91+
next: cx(bem('item', 'next'), userCssClasses.next),
92+
first: cx(bem('item', 'first'), userCssClasses.first),
93+
last: cx(bem('item', 'last'), userCssClasses.last),
94+
active: cx(bem('item', 'active'), userCssClasses.active),
95+
disabled: cx(bem('item', 'disabled'), userCssClasses.disabled)
9496
};
9597

9698
if (maxPages !== undefined) {

0 commit comments

Comments
 (0)