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.
algolia · Nov 10, 2015 · 2ede317 · 2ede317
1 parent 1bbd569
commit 2ede317
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 69 deletions.
diff --git a/components/Pagination/Pagination.js b/components/Pagination/Pagination.js
@@ -23,21 +23,25 @@ class Pagination extends React.Component {
     this.props.setCurrentPage(pageNumber);
   }
 
-  pageLink({label, ariaLabel, pageNumber, className = null, isDisabled = false, isActive = false, createURL}) {
+  pageLink({label, ariaLabel, pageNumber, additionalClassName = null, isDisabled = false, isActive = false, createURL}) {
     let handleClick = this.handleClick.bind(this, pageNumber);
 
+    let cssClasses = {
+      item: cx(this.props.cssClasses.item, additionalClassName),
+      link: cx(this.props.cssClasses.link)
+    };
     if (isDisabled) {
-      className = cx(this.props.cssClasses.disabled, className);
+      cssClasses.item = cx(cssClasses.item, this.props.cssClasses.disabled);
     } else if (isActive) {
-      className = cx(this.props.cssClasses.active, className);
+      cssClasses.item = cx(cssClasses.item, this.props.cssClasses.active);
     }
 
     let url = createURL && !isDisabled ? createURL(pageNumber) : '#';
 
     return (
       <PaginationLink
         ariaLabel={ariaLabel}
-        className={className}
+        cssClasses={cssClasses}
         handleClick={handleClick}
         key={label}
         label={label}
@@ -49,7 +53,7 @@ class Pagination extends React.Component {
   previousPageLink(pager, createURL) {
     return this.pageLink({
       ariaLabel: 'Previous',
-      className: this.props.cssClasses.previous,
+      additionalClassName: this.props.cssClasses.previous,
       isDisabled: pager.isFirstPage(),
       label: this.props.labels.previous,
       pageNumber: pager.currentPage - 1,
@@ -60,7 +64,7 @@ class Pagination extends React.Component {
   nextPageLink(pager, createURL) {
     return this.pageLink({
       ariaLabel: 'Next',
-      className: this.props.cssClasses.next,
+      additionalClassName: this.props.cssClasses.next,
       isDisabled: pager.isLastPage(),
       label: this.props.labels.next,
       pageNumber: pager.currentPage + 1,
@@ -71,7 +75,7 @@ class Pagination extends React.Component {
   firstPageLink(pager, createURL) {
     return this.pageLink({
       ariaLabel: 'First',
-      className: this.props.cssClasses.first,
+      additionalClassName: this.props.cssClasses.first,
       isDisabled: pager.isFirstPage(),
       label: this.props.labels.first,
       pageNumber: 0,
@@ -82,7 +86,7 @@ class Pagination extends React.Component {
   lastPageLink(pager, createURL) {
     return this.pageLink({
       ariaLabel: 'Last',
-      className: this.props.cssClasses.last,
+      additionalClassName: this.props.cssClasses.last,
       isDisabled: pager.isLastPage(),
       label: this.props.labels.last,
       pageNumber: pager.total - 1,
@@ -98,7 +102,7 @@ class Pagination extends React.Component {
 
       pages.push(this.pageLink({
         ariaLabel: pageNumber + 1,
-        className: this.props.cssClasses.page,
+        additionalClassName: this.props.cssClasses.page,
         isActive: isActive,
         label: pageNumber + 1,
         pageNumber: pageNumber,
@@ -135,6 +139,7 @@ Pagination.propTypes = {
   cssClasses: React.PropTypes.shape({
     root: React.PropTypes.string,
     item: React.PropTypes.string,
+    link: React.PropTypes.string,
     page: React.PropTypes.string,
     previous: React.PropTypes.string,
     next: React.PropTypes.string,

diff --git a/components/Pagination/PaginationLink.js b/components/Pagination/PaginationLink.js
@@ -2,13 +2,13 @@ let React = require('react');
 
 class PaginationLink extends React.Component {
   render() {
-    let {className, label, ariaLabel, handleClick, url} = this.props;
+    let {cssClasses, label, ariaLabel, handleClick, url} = this.props;
 
     return (
-      <li className={className}>
+      <li className={cssClasses.item}>
         <a
           ariaLabel={ariaLabel}
-          className={className}
+          className={cssClasses.link}
           dangerouslySetInnerHTML={{__html: label}}
           href={url}
           onClick={handleClick}
@@ -23,7 +23,10 @@ PaginationLink.propTypes = {
     React.PropTypes.string,
     React.PropTypes.number
   ]).isRequired,
-  className: React.PropTypes.string,
+  cssClasses: React.PropTypes.shape({
+    item: React.PropTypes.string,
+    link: React.PropTypes.string
+  }),
   handleClick: React.PropTypes.func.isRequired,
   label: React.PropTypes.oneOfType([
     React.PropTypes.string,

diff --git a/components/Pagination/__tests__/Pagination-test.js b/components/Pagination/__tests__/Pagination-test.js
@@ -48,14 +48,14 @@ describe('Pagination', () => {
   it('should flag the current page as active', () => {
     let out = render({currentPage: 0});
 
-    expect(out.props.children[2][0].props.className).toBe('active page');
-    expect(out.props.children[2][1].props.className).toBe('page');
+    expect(out.props.children[2][0].props.cssClasses.item).toBe('item page active');
+    expect(out.props.children[2][1].props.cssClasses.item).toBe('item page');
   });
 
   it('should disable the first page if already on it', () => {
     let out = render({currentPage: 0, showFirstLast: true});
 
-    expect(out.props.children[0].props.className).toBe('disabled first');
+    expect(out.props.children[0].props.cssClasses.item).toBe('item first disabled');
   });
 
   it('should build the associated URL', () => {
@@ -68,7 +68,7 @@ describe('Pagination', () => {
     expect(out).toEqualJSX(
       <PaginationLink
         ariaLabel={undefined}
-        className={null}
+        cssClasses={{item: '', link: ''}}
         handleClick={() => {}}
         key="test"
         label="test"
@@ -88,7 +88,7 @@ describe('Pagination', () => {
     expect(out).toEqualJSX(
       <PaginationLink
         ariaLabel={undefined}
-        className=""
+        cssClasses={{item: '', link: ''}}
         handleClick={() => {}}
         key="test"
         label="test"
@@ -97,10 +97,10 @@ describe('Pagination', () => {
     expect(createURL.called).toBe(false, 'createURL should not be called');
   });
 
-  it('should disable last first page if already on it', () => {
+  it('should disable last page if already on it', () => {
     let out = render({currentPage: 19, showFirstLast: true});
 
-    expect(out.props.children[4].props.className).toBe('disabled last');
+    expect(out.props.children[4].props.cssClasses.item).toBe('item last disabled');
   });
 
   it('should handle special clicks', () => {

diff --git a/css/default/_pagination.scss b/css/default/_pagination.scss
@@ -8,23 +8,26 @@
       /* disabled pagination item */
       visibility: hidden;
     }
-  }
-  @include element(item-first) {
-    /* first pagination item */
-  }
-  @include element(item-previous) {
-    /* previous pagination item */
-  }
-  @include element(item-page) {
-    /* page */
     @include modifier(active) {
-      /* active page */
+      /* active pagination item */
+    }
+    @include modifier(first) {
+      /* first pagination item */
+    }
+    @include modifier(previous) {
+      /* previous pagination item */
+    }
+    @include modifier(page) {
+      /* page pagination item */
+    }
+    @include modifier(next) {
+      /* next pagination item */
+    }
+    @include modifier(last) {
+      /* last pagination item */
     }
   }
-  @include element(item-next) {
-    /* next pagination item */
-  }
-  @include element(item-last) {
-    /* last pagination item */
+  @include element(link) {
+    /* pagination link */
   }
 }
diff --git a/docs/_includes/widget-jsdoc/pagination.md b/docs/_includes/widget-jsdoc/pagination.md
@@ -4,6 +4,7 @@
 |  <span class='attr-optional'>`options.cssClasses`</span> | CSS classes to be added |
 |  <span class='attr-optional'>`options.cssClasses.root`</span> | CSS classes added to the parent <ul> |
 |  <span class='attr-optional'>`options.cssClasses.item`</span> | CSS classes added to each <li> |
+|  <span class='attr-optional'>`options.cssClasses.link`</span> | CSS classes added to each link |
 |  <span class='attr-optional'>`options.cssClasses.page`</span> | CSS classes added to page <li> |
 |  <span class='attr-optional'>`options.cssClasses.previous`</span> | CSS classes added to the previous <li> |
 |  <span class='attr-optional'>`options.cssClasses.next`</span> | CSS classes added to the next <li> |

diff --git a/docs/examples/airbnb/scss/_pagination.scss b/docs/examples/airbnb/scss/_pagination.scss
@@ -3,15 +3,13 @@
 
 @include block(pagination) {
   @include element(item) {
-    @include modifier(disabled) {
-      visibility: hidden;
-    }
+    @include modifier(disabled)
+    @include modifier(active)
+    @include modifier(first)
+    @include modifier(previous)
+    @include modifier(page)
+    @include modifier(next)
+    @include modifier(last)
   }
-  @include element(item-first);
-  @include element(item-previous);
-  @include element(item-page) {
-    @include modifier(active);
-  }
-  @include element(item-next);
-  @include element(item-last);
+  @include element(link)
 }
diff --git a/docs/examples/youtube/scss/_pagination.scss b/docs/examples/youtube/scss/_pagination.scss
@@ -3,15 +3,14 @@
 
 @include block(pagination) {
   @include element(item) {
-    @include modifier(disabled) {
-      visibility: hidden;
-    }
+    @include modifier(disabled)
+    @include modifier(active)
+    @include modifier(first)
+    @include modifier(previous)
+    @include modifier(page)
+    @include modifier(next)
+    @include modifier(last)
   }
-  @include element(item-first);
-  @include element(item-previous);
-  @include element(item-page) {
-    @include modifier(active);
-  }
-  @include element(item-next);
-  @include element(item-last);
+  @include element(link)
 }
+
diff --git a/widgets/pagination/__tests__/pagination-test.js b/widgets/pagination/__tests__/pagination-test.js
@@ -30,6 +30,7 @@ describe('pagination()', () => {
     cssClasses = {
       root: 'root',
       item: 'item',
+      link: 'link',
       page: 'page',
       previous: 'previous',
       next: 'next',
@@ -107,13 +108,14 @@ describe('pagination()', () => {
       cssClasses: {
         root: 'ais-pagination root',
         item: 'ais-pagination--item item',
-        page: 'ais-pagination--item ais-pagination--item-page page',
-        previous: 'ais-pagination--item ais-pagination--item-previous previous',
-        next: 'ais-pagination--item ais-pagination--item-next next',
-        first: 'ais-pagination--item ais-pagination--item-first first',
-        last: 'ais-pagination--item ais-pagination--item-last last',
-        active: 'ais-pagination--item  ais-pagination--item-page ais-pagination--item-page__active active',
-        disabled: 'ais-pagination--item ais-pagination--item__disabled disabled'
+        link: 'ais-pagination--link link',
+        page: 'ais-pagination--item__page page',
+        previous: 'ais-pagination--item__previous previous',
+        next: 'ais-pagination--item__next next',
+        first: 'ais-pagination--item__first first',
+        last: 'ais-pagination--item__last last',
+        active: 'ais-pagination--item__active active',
+        disabled: 'ais-pagination--item__disabled disabled'
       },
       currentPage: 0,
       shouldAutoHideContainer: false,

diff --git a/widgets/pagination/pagination.js b/widgets/pagination/pagination.js
@@ -20,6 +20,7 @@ let defaultLabels = {
  * @param  {Object} [options.cssClasses] CSS classes to be added
  * @param  {string} [options.cssClasses.root] CSS classes added to the parent <ul>
  * @param  {string} [options.cssClasses.item] CSS classes added to each <li>
+ * @param  {string} [options.cssClasses.link] CSS classes added to each link
  * @param  {string} [options.cssClasses.page] CSS classes added to page <li>
  * @param  {string} [options.cssClasses.previous] CSS classes added to the previous <li>
  * @param  {string} [options.cssClasses.next] CSS classes added to the next <li>
@@ -84,13 +85,14 @@ function pagination({
       let cssClasses = {
         root: cx(bem(null), userCssClasses.root),
         item: cx(bem('item'), userCssClasses.item),
-        page: cx(bem('item'), bem('item-page'), userCssClasses.page),
-        previous: cx(bem('item'), bem('item-previous'), userCssClasses.previous),
-        next: cx(bem('item'), bem('item-next'), userCssClasses.next),
-        first: cx(bem('item'), bem('item-first'), userCssClasses.first),
-        last: cx(bem('item'), bem('item-last'), userCssClasses.last),
-        active: cx(bem('item'), bem('item-page'), bem('item-page', 'active'), userCssClasses.active),
-        disabled: cx(bem('item'), bem('item', 'disabled'), userCssClasses.disabled)
+        link: cx(bem('link'), userCssClasses.link),
+        page: cx(bem('item', 'page'), userCssClasses.page),
+        previous: cx(bem('item', 'previous'), userCssClasses.previous),
+        next: cx(bem('item', 'next'), userCssClasses.next),
+        first: cx(bem('item', 'first'), userCssClasses.first),
+        last: cx(bem('item', 'last'), userCssClasses.last),
+        active: cx(bem('item', 'active'), userCssClasses.active),
+        disabled: cx(bem('item', 'disabled'), userCssClasses.disabled)
       };
 
       if (maxPages !== undefined) {