Change pagination UX to show current and max page number (#21)

* Show current page number in Pagination component

* Change loading indicator UI/UX to rely on fetching state

In the previous implementation, show a loading indicator infinitely when
API server response has no items. So, I improve this.

app/components/HackerNewsList/HackerNewsList.scss

@@ -6,6 +6,10 @@
   }
 }
 
+.HackerNewsList__noti {
+  text-align: center;
+}
+
 .HackerNewsList__item {
   margin: 30px 0;
 }

app/components/HackerNewsList/HackerNewsList.spec.jsx

@@ -40,4 +40,9 @@ describe('<HackerNewsList />', () => {
     const wrapper = shallow(<HackerNewsList feeds={feeds} />);
     expect(wrapper).toMatchSnapshot();
   });
+
+  it('should have .HackerNewsList__noti when have no items', () => {
+    const wrapper = shallow(<HackerNewsList feeds={Immutable.List()} isFetching={false} />);
+    expect(wrapper.find('.HackerNewsList__noti')).toHaveLength(1);
+  });
 });

app/components/HackerNewsList/HackerNewsList.stories.jsx

@@ -13,7 +13,9 @@ stories
       {story()}
     </MemoryRouter>
   ))
-  .add('default', () => (
+  .add('fetching data', () => <HackerNewsList isFetching />)
+  .add('no data', () => <HackerNewsList feeds={Immutable.List()} isFetching={false} />)
+  .add('with data', () => (
     <HackerNewsList
       feeds={Immutable.fromJS([
         {

app/components/HackerNewsList/__snapshots__/HackerNewsList.spec.jsx.snap

@@ -5,7 +5,7 @@ exports[`<HackerNewsList /> should match snapshot when render default 1`] = `
   className="HackerNewsList"
 >
   <LoadingIndicator
-    active={true}
+    active={false}
     style={
       Object {
         "left": "calc(50% - 24px)",
@@ -14,6 +14,11 @@ exports[`<HackerNewsList /> should match snapshot when render default 1`] = `
       }
     }
   />
+  <p
+    className="HackerNewsList__noti"
+  >
+    There are no items to show.
+  </p>
 </ul>
 `;
 

app/components/HackerNewsList/index.jsx

@@ -9,20 +9,25 @@ import './HackerNewsList.scss';
 
 const propTypes = {
   feeds: PropTypes.object, // eslint-disable-line react/forbid-prop-types
+  isFetching: PropTypes.bool,
 };
 
 const defaultProps = {
   feeds: Immutable.List(),
+  isFetching: false,
 };
 
-function HackerNewsList({ feeds }) {
+function HackerNewsList({ feeds, isFetching }) {
+  const haveNoItems = !isFetching && feeds.size === 0;
+
   return (
     <ul className="HackerNewsList">
       <LoadingIndicator
-        active={!feeds.size}
+        active={isFetching}
         style={{ position: 'absolute', left: 'calc(50% - 24px)', marginTop: '10px' }}
       />
-      {feeds.map((feed, index) => (
+      {haveNoItems && <p className="HackerNewsList__noti">There are no items to show.</p>}
+      {!isFetching && feeds.map((feed, index) => (
         <li className="HackerNewsList__item" key={feed.get('id')}>
           <span className="HackerNewsList__index">{index + 1}</span>
           <HackerNewsListItem {...feed.toJS()} />

app/components/Pagination/Pagination.spec.jsx

@@ -9,6 +9,18 @@ describe('<Pagination />', () => {
     expect(wrapper).toMatchSnapshot();
   });
 
+  it('should have .Pagination__button--disabled when props.currentPage === 1', () => {
+    const wrapper = shallow(<Pagination currentPage={1} />);
+    const button = wrapper.find('.Pagination__button--disabled');
+    expect(button.props()['data-button']).toBe('prev');
+  });
+
+  it('should have .Pagination__button--disabled when props.currentPage === 10', () => {
+    const wrapper = shallow(<Pagination currentPage={10} />);
+    const button = wrapper.find('.Pagination__button--disabled');
+    expect(button.props()['data-button']).toBe('next');
+  });
+
   it('should calls onPaginate with currentPage - 1 when simulate prev button click event', () => {
     const currentPage = 3;
     const onPaginate = jest.fn();
@@ -20,8 +32,8 @@ describe('<Pagination />', () => {
       />
     ));
     wrapper.find('.Pagination__button').at(0).simulate('click');
-    expect(onPaginate.mock.calls.length).toBe(1);
-    expect(onPaginate.mock.calls[0]).toEqual([currentPage - 1]);
+    expect(onPaginate).toHaveBeenCalledTimes(1);
+    expect(onPaginate).toHaveBeenCalledWith(currentPage - 1);
   });
 
   it('should calls onPaginate with currentPage + 1 when simulate next button click event', () => {
@@ -35,7 +47,7 @@ describe('<Pagination />', () => {
       />
     ));
     wrapper.find('.Pagination__button').at(1).simulate('click');
-    expect(onPaginate.mock.calls.length).toBe(1);
-    expect(onPaginate.mock.calls[0]).toEqual([currentPage + 1]);
+    expect(onPaginate).toHaveBeenCalledTimes(1);
+    expect(onPaginate).toHaveBeenCalledWith(currentPage + 1);
   });
 });

app/components/Pagination/__snapshots__/Pagination.spec.jsx.snap

@@ -9,16 +9,21 @@ exports[`<Pagination /> should match snapshot when render default 1`] = `
   >
     <button
       className="Pagination__button Pagination__button--disabled"
+      data-button="prev"
       disabled={true}
       onClick={[Function]}
       type="button"
     >
       &lt;
        Prev
     </button>
+    1
+     / 10
      
     <button
       className="Pagination__button"
+      data-button="next"
+      disabled={false}
       onClick={[Function]}
       type="button"
     >

app/components/Pagination/index.jsx

@@ -16,25 +16,34 @@ const defaultProps = {
 
 function Pagination({ currentPage, onPaginate }) {
   const prevDisabled = currentPage === 1;
+  const nextDisabled = currentPage === 10;
 
   const prevButtonClassName = cx('Pagination__button', {
     'Pagination__button--disabled': prevDisabled,
   });
 
+  const nextButtonClassName = cx('Pagination__button', {
+    'Pagination__button--disabled': nextDisabled,
+  });
+
   return (
     <div className="Pagination">
       <div className="Pagination__inner">
         <button
           className={prevButtonClassName}
+          data-button="prev"
           disabled={prevDisabled}
           type="button"
           onClick={() => onPaginate(currentPage - 1)}
         >
           {'<'} Prev
         </button>
+        {currentPage} / 10
         {' '}
         <button
-          className="Pagination__button"
+          className={nextButtonClassName}
+          data-button="next"
+          disabled={nextDisabled}
           type="button"
           onClick={() => onPaginate(currentPage + 1)}
         >

app/containers/HackerNewsListContainer.jsx

@@ -1,10 +1,11 @@
 import { connect } from 'react-redux';
 
 import HackerNewsList from 'components/HackerNewsList';
-import { getFeeds } from 'store/selectors';
+import { getFeeds, getIsFetching } from 'store/selectors';
 
 const mapStateToProps = (state, props) => ({
   feeds: getFeeds(state, props),
+  isFetching: getIsFetching(state),
 });
 
 export default connect(mapStateToProps)(HackerNewsList);

app/store/__tests__/reducers/isFetching.spec.js

@@ -0,0 +1,62 @@
+import * as ACTIONS from 'store/actionTypes';
+import isFetching from 'store/isFetching';
+
+describe('isFetching reducer', () => {
+  it('should return the initial state', () => {
+    expect(isFetching(undefined, {})).toBe(false);
+  });
+
+  it('should handle HACKER_COMMENTS_FETCH_REQUEST', () => {
+    expect(isFetching(false, {
+      type: ACTIONS.HACKER_COMMENTS_FETCH_REQUEST,
+    })).toBe(true);
+  });
+
+  it('should handle HACKER_COMMENTS_FETCH_SUCCESS', () => {
+    expect(isFetching(true, {
+      type: ACTIONS.HACKER_COMMENTS_FETCH_SUCCESS,
+    })).toBe(false);
+  });
+
+  it('should handle HACKER_COMMENTS_FETCH_FAILURE', () => {
+    expect(isFetching(true, {
+      type: ACTIONS.HACKER_COMMENTS_FETCH_FAILURE,
+    })).toBe(false);
+  });
+
+  it('should handle HACKER_NEWS_FETCH_REQUEST', () => {
+    expect(isFetching(false, {
+      type: ACTIONS.HACKER_NEWS_FETCH_REQUEST,
+    })).toBe(true);
+  });
+
+  it('should handle HACKER_NEWS_FETCH_SUCCESS', () => {
+    expect(isFetching(true, {
+      type: ACTIONS.HACKER_NEWS_FETCH_SUCCESS,
+    })).toBe(false);
+  });
+
+  it('should handle HACKER_NEWS_FETCH_FAILURE', () => {
+    expect(isFetching(true, {
+      type: ACTIONS.HACKER_NEWS_FETCH_FAILURE,
+    })).toBe(false);
+  });
+
+  it('should handle HACKER_USER_FETCH_REQUEST', () => {
+    expect(isFetching(false, {
+      type: ACTIONS.HACKER_USER_FETCH_REQUEST,
+    })).toBe(true);
+  });
+
+  it('should handle HACKER_USER_FETCH_SUCCESS', () => {
+    expect(isFetching(true, {
+      type: ACTIONS.HACKER_USER_FETCH_SUCCESS,
+    })).toBe(false);
+  });
+
+  it('should handle HACKER_USER_FETCH_FAILURE', () => {
+    expect(isFetching(true, {
+      type: ACTIONS.HACKER_USER_FETCH_FAILURE,
+    })).toBe(false);
+  });
+});

app/store/__tests__/selectors.spec.js

@@ -9,6 +9,7 @@ describe('selectors', () => {
       posts: Immutable.Map(),
     },
     currentPage: 1,
+    isFetching: false,
     items: {
       ask: {},
       jobs: {},
@@ -27,6 +28,10 @@ describe('selectors', () => {
     expect(selectors.getCurrentPage(state)).toBe(state.get('currentPage'));
   });
 
+  it('should select isFetching', () => {
+    expect(selectors.getIsFetching(state)).toBe(state.get('isFetching'));
+  });
+
   it('should select items', () => {
     expect(selectors.getItems(state)).toEqual(state.get('items'));
   });

app/store/isFetching/index.js

@@ -0,0 +1,21 @@
+import * as ACTIONS from '../actionTypes';
+
+function itemsReducer(state = false, action) {
+  switch (action.type) {
+    case ACTIONS.HACKER_COMMENTS_FETCH_REQUEST:
+    case ACTIONS.HACKER_NEWS_FETCH_REQUEST:
+    case ACTIONS.HACKER_USER_FETCH_REQUEST:
+      return true;
+    case ACTIONS.HACKER_COMMENTS_FETCH_SUCCESS:
+    case ACTIONS.HACKER_COMMENTS_FETCH_FAILURE:
+    case ACTIONS.HACKER_NEWS_FETCH_SUCCESS:
+    case ACTIONS.HACKER_NEWS_FETCH_FAILURE:
+    case ACTIONS.HACKER_USER_FETCH_SUCCESS:
+    case ACTIONS.HACKER_USER_FETCH_FAILURE:
+      return false;
+    default:
+      return state;
+  }
+}
+
+export default itemsReducer;

app/store/rootReducer.js

@@ -4,13 +4,15 @@ import { routerReducer } from 'react-router-redux';
 import byId from './byId';
 import comments from './comments';
 import currentPage from './currentPage';
+import isFetching from './isFetching';
 import items from './items';
 import user from './user';
 
 const rootReducer = combineReducers({
   byId,
   comments,
   currentPage,
+  isFetching,
   items,
   user,
   router: routerReducer,

app/store/selectors.js

@@ -3,6 +3,7 @@ import { createSelector } from 'reselect';
 
 export const getById = state => state.get('byId');
 export const getCurrentPage = state => state.get('currentPage');
+export const getIsFetching = state => state.get('isFetching');
 export const getItems = state => state.get('items');
 export const getUser = state => state.get('user');