Improve pagination component (#566)

* Add isLoading prop to Pagination

* Add keyboard navigation (j, k)

* Add number formatting for pagination

* Leverage isDefined and isNumber when formatting a number

* Add coverage test to util/number

src/components/KeypressListener/__tests__/KeypressListener.test.js

@@ -3,7 +3,7 @@ import { mount } from 'enzyme'
 import KeypressListener from '..'
 import Keys from '../../../constants/Keys'
 
-const simulateKeyPress = (keyCode, modifier) => {
+export const simulateKeyPress = (keyCode, modifier) => {
   const event = new Event('keyup')
   event.keyCode = keyCode
   if (modifier) {

src/components/Pagination/Pagination.tsx

@@ -6,6 +6,9 @@ import { namespaceComponent } from '../../utilities/component'
 import { noop } from '../../utilities/other'
 import { COMPONENT_KEY } from './Pagination.utils'
 import pluralize from '../../utilities/pluralize'
+import KeypressListener from '../KeypressListener'
+import Keys from '../../constants/Keys'
+import { formatNumber } from '../../utilities/number'
 
 import {
   PaginationUI,
@@ -20,6 +23,7 @@ import Icon from '../Icon'
 export interface Props {
   activePage: number
   className?: string
+  isLoading?: boolean
   innerRef: (node: HTMLElement) => void
   onChange: (nextPageNumber: number) => void
   rangePerPage: number
@@ -33,6 +37,7 @@ export interface Props {
 export class Pagination extends React.PureComponent<Props> {
   static defaultProps = {
     activePage: 1,
+    isLoading: false,
     innerRef: noop,
     onChange: noop,
     rangePerPage: 50,
@@ -120,17 +125,19 @@ export class Pagination extends React.PureComponent<Props> {
   renderRange() {
     const { separator, subject } = this.props
     const totalItems = this.getTotalItems()
-    const totalNode = <span className="c-Pagination__total">{totalItems}</span>
+    const totalNode = (
+      <span className="c-Pagination__total">{formatNumber(totalItems)}</span>
+    )
 
     if (!totalItems) {
       return subject ? totalNode : null
     }
 
     return (
       <Text className="c-Pagination__range">
-        <RangeUI>{this.getStartRange()}</RangeUI>
+        <RangeUI>{formatNumber(this.getStartRange())}</RangeUI>
         {` `}-{` `}
-        <RangeUI>{this.getEndRange()}</RangeUI>
+        <RangeUI>{formatNumber(this.getEndRange())}</RangeUI>
         {` `}
         {separator}
         {` `}
@@ -140,18 +147,33 @@ export class Pagination extends React.PureComponent<Props> {
   }
 
   renderNavigation() {
+    const { isLoading } = this.props
     const currentPage = this.getCurrentPage()
     const isNotFirstPage = currentPage > 1
     const isLastPage = currentPage >= this.getNumberOfPages()
 
     return (
       <NavigationUI>
+        <KeypressListener
+          keyCode={Keys.KEY_J}
+          handler={this.handlePrevClick}
+          noModifier
+          type="keyup"
+        />
+        <KeypressListener
+          keyCode={Keys.KEY_K}
+          handler={this.handleNextClick}
+          noModifier
+          type="keyup"
+        />
         {isNotFirstPage && [
           <ButtonIconUI
             key="firstButton"
             version={2}
             onClick={this.handleFirstClick}
             className="c-Pagination__firstButton"
+            disabled={isLoading}
+            title="First page"
           >
             <Icon name="arrow-left-double-large" size="24" center />
           </ButtonIconUI>,
@@ -160,24 +182,28 @@ export class Pagination extends React.PureComponent<Props> {
             version={2}
             onClick={this.handlePrevClick}
             className="c-Pagination__prevButton"
+            disabled={isLoading}
+            title="Previous page (j)"
           >
             <Icon name="arrow-left-single-large" size="24" center />
           </ButtonIconUI>,
         ]}
 
         <ButtonIconUI
           version={2}
-          disabled={isLastPage}
+          disabled={isLastPage || isLoading}
           onClick={this.handleNextClick}
           className="c-Pagination__nextButton"
+          title="Next page (k)"
         >
           <Icon name="arrow-right-single-large" size="24" center />
         </ButtonIconUI>
         <ButtonIconUI
           version={2}
-          disabled={isLastPage}
+          disabled={isLastPage || isLoading}
           onClick={this.handleEndClick}
           className="c-Pagination__lastButton"
+          title="Last page"
         >
           <Icon name="arrow-right-double-large" size="24" center />
         </ButtonIconUI>

src/components/Pagination/__tests__/Pagination.test.js

@@ -3,7 +3,8 @@ import { mount } from 'enzyme'
 import { Pagination } from '../Pagination'
 import { NavigationUI } from '../Pagination.css'
 import { hasClass } from '../../../tests/helpers/enzyme'
-
+import { simulateKeyPress } from '../../KeypressListener/__tests__/KeypressListener.test'
+import Keys from '../../../constants/Keys'
 describe('className', () => {
   test('Has a default className', () => {
     const wrapper = mount(<Pagination />)
@@ -155,6 +156,28 @@ describe('Navigation', () => {
     expect(last.props('disabled')).toBeTruthy()
   })
 
+  test('Disables all buttons when loading', () => {
+    const wrapper = mount(
+      <Pagination
+        isLoading={true}
+        showNavigation={true}
+        totalItems={15}
+        rangePerPage={5}
+        activePage={3}
+      />
+    )
+
+    const first = wrapper.find('Button.c-Pagination__firstButton').first()
+    const prev = wrapper.find('Button.c-Pagination__prevButton').first()
+    const next = wrapper.find('Button.c-Pagination__nextButton').first()
+    const last = wrapper.find('Button.c-Pagination__lastButton').first()
+
+    expect(first.props('disabled')).toBeTruthy()
+    expect(prev.props('disabled')).toBeTruthy()
+    expect(next.props('disabled')).toBeTruthy()
+    expect(last.props('disabled')).toBeTruthy()
+  })
+
   test('Sets 1 as the minimum for currentPage', () => {
     const wrapper = mount(
       <Pagination
@@ -384,3 +407,41 @@ describe('Clicks', () => {
     expect(changeWatcher).not.toHaveBeenCalled()
   })
 })
+
+describe('Keyboard', () => {
+  test('Pressing j navigate to previous page', () => {
+    const totalItems = 17
+    const rangePerPage = 5
+    const changeWatcher = jest.fn()
+
+    mount(
+      <Pagination
+        showNavigation={true}
+        totalItems={totalItems}
+        rangePerPage={rangePerPage}
+        activePage={2}
+        onChange={changeWatcher}
+      />
+    )
+    simulateKeyPress(Keys.KEY_J)
+    expect(changeWatcher).toHaveBeenCalledWith(1)
+  })
+
+  test('Pressing k navigate to next page', () => {
+    const totalItems = 17
+    const rangePerPage = 5
+    const changeWatcher = jest.fn()
+
+    mount(
+      <Pagination
+        showNavigation={true}
+        totalItems={totalItems}
+        rangePerPage={rangePerPage}
+        activePage={1}
+        onChange={changeWatcher}
+      />
+    )
+    simulateKeyPress(Keys.KEY_K)
+    expect(changeWatcher).toHaveBeenCalledWith(2)
+  })
+})

src/utilities/__tests__/number.test.js

@@ -1,4 +1,4 @@
-import { isEven, isOdd, getMiddleIndex } from '../number'
+import { isEven, isOdd, getMiddleIndex, formatNumber } from '../number'
 
 describe('isEven', () => {
   test('Should detect even numbers', () => {
@@ -39,3 +39,16 @@ describe('getMiddleIndex', () => {
     expect(getMiddleIndex(100)).toBe(49)
   })
 })
+
+describe('formatNumber', () => {
+  test('Should format number', () => {
+    expect(formatNumber(4)).toBe('4')
+    expect(formatNumber(5000)).toBe('5,000')
+    expect(formatNumber(4500.23)).toBe('4,500.23')
+    expect(formatNumber(0)).toBe('0')
+    expect(formatNumber(null)).toBe(null)
+    expect(formatNumber(-10028.444)).toBe('-10,028.444')
+    expect(formatNumber(1032234234028.4)).toBe('1,032,234,234,028.4')
+    expect(formatNumber('test')).toBe('test')
+  })
+})

src/utilities/number.js

@@ -1,3 +1,5 @@
+import { isDefined, isNumber } from './is'
+
 export const isEven = (number: string): boolean => number % 2 === 0
 
 export const isOdd = (number: string): boolean => !isEven(number)
@@ -7,3 +9,10 @@ export const getMiddleIndex = (number: string): number => {
 
   return isOdd(number) ? middle : middle - 1
 }
+
+export const formatNumber = num => {
+  if (!isDefined(num)) return num
+  if (!isNumber(num)) return num
+
+  return num.toString().replace(/(\d)(?=(\d{3})+(?!\d))/g, '$1,')
+}

stories/Pagination.stories.js

@@ -94,6 +94,20 @@ stories.add('end of navigation', () => {
   )
 })
 
+stories.add('isLoading', () => {
+  return (
+    <Pagination
+      {...getKnobsProps({
+        subject: subject(),
+        activePage: activePage(10),
+        isLoading: boolean('isLoading', true),
+      })}
+      showNavigation={true}
+      onChange={action('changePage')}
+    />
+  )
+})
+
 class PaginationWrapper extends React.Component {
   state = {
     activePage: 1,