-
Notifications
You must be signed in to change notification settings - Fork 1.6k
add solution #1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
add solution #1437
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,40 @@ | ||
| import React from 'react'; | ||
| import React, { useState } from 'react'; | ||
| import './App.css'; | ||
| import { getNumbers } from './utils'; | ||
| import { Pagination } from './components/Pagination'; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const items = getNumbers(1, 42).map(n => `Item ${n}`); | ||
|
|
||
| export const App: React.FC = () => { | ||
| const [perPage, setPerPage] = useState(5); | ||
| const [currentPage, setCurrentPage] = useState(1); | ||
|
|
||
| const start = (currentPage - 1) * perPage; | ||
| const end = start + perPage; | ||
|
|
||
| const visibleItems = items.slice(start, end); | ||
|
|
||
| return ( | ||
| <div className="container"> | ||
| <h1>Items with Pagination</h1> | ||
|
|
||
| <p className="lead" data-cy="info"> | ||
| Page 1 (items 1 - 5 of 42) | ||
| {`Page ${currentPage} (items ${start + 1} - ${Math.min(end, items.length)} of ${items.length})`} | ||
| </p> | ||
|
|
||
| <div className="form-group row"> | ||
| <div className="col-3 col-sm-2 col-xl-1"> | ||
| <select | ||
| data-cy="perPageSelector" | ||
| id="perPageSelector" | ||
| className="form-control"> | ||
| className="form-control" | ||
| value={perPage} | ||
| onChange={event => { | ||
| setPerPage(+event.target.value); | ||
| setCurrentPage(1); | ||
| }} | ||
| > | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check has the same potential issue as the |
||
| <option value="3">3</option> | ||
| <option value="5">5</option> | ||
| <option value="10">10</option> | ||
|
|
@@ -32,78 +47,20 @@ export const App: React.FC = () => { | |
| </label> | ||
| </div> | ||
|
|
||
| {/* Move this markup to Pagination */} | ||
| <ul className="pagination"> | ||
| <li className="page-item disabled"> | ||
| <a | ||
| data-cy="prevLink" | ||
| className="page-link" | ||
| href="#prev" | ||
| aria-disabled="true"> | ||
| « | ||
| </a> | ||
| </li> | ||
| <li className="page-item active"> | ||
| <a data-cy="pageLink" className="page-link" href="#1"> | ||
| 1 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#2"> | ||
| 2 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#3"> | ||
| 3 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#4"> | ||
| 4 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#5"> | ||
| 5 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#6"> | ||
| 6 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#7"> | ||
| 7 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#8"> | ||
| 8 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a data-cy="pageLink" className="page-link" href="#9"> | ||
| 9 | ||
| </a> | ||
| </li> | ||
| <li className="page-item"> | ||
| <a | ||
| data-cy="nextLink" | ||
| className="page-link" | ||
| href="#next" | ||
| aria-disabled="false"> | ||
| » | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| <Pagination | ||
| total={items.length} | ||
| perPage={perPage} | ||
| currentPage={currentPage} | ||
| onPageChange={page => { | ||
| setCurrentPage(page); | ||
| }} | ||
| /> | ||
| <ul> | ||
| <li data-cy="item">Item 1</li> | ||
| <li data-cy="item">Item 2</li> | ||
| <li data-cy="item">Item 3</li> | ||
| <li data-cy="item">Item 4</li> | ||
| <li data-cy="item">Item 5</li> | ||
| {visibleItems.map(item => ( | ||
| <li data-cy="item" key={item}> | ||
| {item} | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,92 @@ | ||
| export const Pagination = () => {}; | ||
| import classNames from 'classnames'; | ||
| import React from 'react'; | ||
|
|
||
| type Props = { | ||
| total: number; | ||
| perPage: number; | ||
| currentPage?: number; | ||
| onPageChange: (value: number) => void; | ||
| }; | ||
|
|
||
| export const Pagination: React.FC<Props> = ({ | ||
| total, | ||
| perPage, | ||
| currentPage = 1, | ||
| onPageChange, | ||
| }) => { | ||
| const pages = Array.from( | ||
| { length: Math.ceil(total / perPage) }, | ||
| (_, i) => i + 1, | ||
| ); | ||
|
|
||
| const totalPages = Math.ceil(total / perPage); | ||
|
|
||
| return ( | ||
| <ul className="pagination"> | ||
| <li | ||
| className={classNames('page-item', { | ||
| disabled: currentPage <= 1, | ||
| })} | ||
| > | ||
| <a | ||
| data-cy="prevLink" | ||
| className="page-link" | ||
| href="#prev" | ||
| aria-disabled={currentPage === pages[0]} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition can cause issues when |
||
| onClick={event => { | ||
| event.preventDefault(); | ||
| if (currentPage !== pages[0]) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check has the same issue as the |
||
| onPageChange(currentPage - 1); | ||
| } | ||
| }} | ||
| > | ||
| « | ||
| </a> | ||
| </li> | ||
|
|
||
| {pages.map(page => { | ||
| return ( | ||
| <li | ||
| className={classNames('page-item', { | ||
| active: page === currentPage, | ||
| })} | ||
| key={page} | ||
| > | ||
| <a | ||
| data-cy="pageLink" | ||
| className="page-link" | ||
| href={`#${page}`} | ||
| onClick={() => { | ||
| if (page !== currentPage) { | ||
| onPageChange(page); | ||
| } | ||
| }} | ||
| > | ||
| {page} | ||
| </a> | ||
| </li> | ||
| ); | ||
| })} | ||
| <li | ||
| className={classNames('page-item', { | ||
| disabled: currentPage >= totalPages, | ||
| })} | ||
| > | ||
| <a | ||
| data-cy="nextLink" | ||
| className="page-link" | ||
| href="#next" | ||
| aria-disabled={currentPage === pages[pages.length - 1]} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the 'previous' button, this condition is not safe for the case where |
||
| onClick={event => { | ||
| event.preventDefault(); | ||
| if (currentPage !== pages[pages.length - 1]) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition also fails when the |
||
| onPageChange(currentPage + 1); | ||
| } | ||
| }} | ||
| > | ||
| » | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| ); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition for
aria-disabledcan cause issues whentotalis 0, aspages[0]would beundefined. To make it more robust and consistent with theli'sdisabledclass, consider usingcurrentPage <= 1.