Skip to content
Open

Dev #1434

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 53 additions & 79 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,59 @@
import React from 'react';
import React, { useState } from 'react';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app must read and write the page and perPage query params via React Router so reloading restores the current view. Import and use appropriate router hooks (e.g. useSearchParams in react-router-dom v6 or useLocation/useHistory in v5) and useEffect to 1) read ?page and ?perPage on mount and initialize state, and 2) update the URL whenever currentPage or perPage change. This addresses the required behavior to save ?page=2&perPage=7 in the URL (checklist item #13).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import React Router hooks and useEffect here. You need useSearchParams (from react-router-dom) and useEffect (from React) so the app can read/write page and perPage in the URL. This is required by the task's (*) URL persistence requirement.

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}`);
const totalItems = 42;
const items = getNumbers(1, totalItems).map(n => `Item ${n}`);

enum NumberPerPageItems {
THREE = 3,
FIVE = 5,
TEN = 10,
TWENTY = 20,
}

export const App: React.FC = () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app currently doesn't persist page and perPage in the URL or read them on load. The task's checklist includes using React Router to save/apply ?page=2&perPage=7 in the URL (checklist item #13). Add URL-sync (read from query on mount and update query params when page/perPage change) so the app restores state from the URL on page load.

const [currentPage, setCurrentPage] = useState(1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't initialize currentPage as a fixed 1 — read it from the URL search params on mount (fallback to 1). Use parsed numeric value from searchParams.get('page') (e.g. Number(searchParams.get('page') ?? '1')).

const [perPage, setPerPage] = useState(NumberPerPageItems.FIVE);
Comment on lines +18 to +19
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial perPage and currentPage state are hard-coded here. Instead initialize them from the URL query params (if present) on mount or compute initial state from location.search so the app applies ?page=...&perPage=... on page load. Use parseInt with sensible fallbacks (1 and 5).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for perPage: initialize from the URL search params with fallback to NumberPerPageItems.FIVE. Parse to number (e.g. Number(searchParams.get('perPage') ?? String(NumberPerPageItems.FIVE))).


const startItem = currentPage * perPage - (perPage - 1);
const endItem = currentPage * perPage;

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 {startItem} -{' '}
{endItem > totalItems ? totalItems : endItem} of {totalItems})
</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">
<option value="3">3</option>
<option value="5">5</option>
<option value="10">10</option>
<option value="20">20</option>
className="form-control"
onChange={e => {
setPerPage(Number(e.target.value));
setCurrentPage(1);
Comment on lines +39 to +41
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the <select> changes you reset the page to 1 (correct). Also make sure the URL is updated to ?page=1&perPage=.... Recommended: centralize URL updates in a useEffect that writes search params whenever currentPage or perPage change, rather than updating the URL here and in other handlers.

}}
value={perPage}
>
<option value={NumberPerPageItems.THREE}>
{NumberPerPageItems.THREE}
</option>
<option value={NumberPerPageItems.FIVE}>
{NumberPerPageItems.FIVE}
</option>
<option value={NumberPerPageItems.TEN}>
{NumberPerPageItems.TEN}
</option>
<option value={NumberPerPageItems.TWENTY}>
{NumberPerPageItems.TWENTY}
</option>
</select>
</div>

Expand All @@ -32,78 +62,22 @@ 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={totalItems} // total number of items to paginate
perPage={perPage} // number of items per page
currentPage={currentPage} /* optional with 1 by default */
onPageChange={page => {
if (currentPage !== page) {
setCurrentPage(page);
Comment on lines +69 to +71
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onPageChange handler updates currentPage (good). Ensure that this change is persisted to the URL as well (either by calling setSearchParams here or — recommended — letting a useEffect that watches currentPage/perPage do it). This is necessary so reloading restores the same page.

}
}}
Comment on lines +69 to +73
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the user changes page via onPageChange you update the local state correctly, but you also need to update the URL query params at that time so the current page is reflected in the address bar (and survives reload). Add code (e.g. in the same handler or in a useEffect watching currentPage/perPage) to push the new ?page=...&perPage=... to history. This satisfies the requirement that App listens to page changes and persists them to the URL (checklist item #13).

/>
<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>
{items.slice(startItem - 1, endItem).map(i => (
<li key={i} data-cy="item">
{i}
</li>
))}
</ul>
</div>
);
Expand Down
93 changes: 92 additions & 1 deletion src/components/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
@@ -1 +1,92 @@
export const Pagination = () => {};
import classNames from 'classnames';

type Props = {
total: number;
perPage: number;
currentPage?: number;
onPageChange: (page: number) => void;
};

export const Pagination: React.FC<Props> = ({
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You use the React.FC type on line 10 but React (or FC) is not imported in this file. Add an import at the top such as import React from 'react' or import type { FC } from 'react' and then use FC<Props> to avoid a TypeScript/compile error.

total,
perPage,
currentPage = 1,
onPageChange,
}) => {
const numberOfPage = Math.ceil(total / perPage);

const pagesList = Array.from(
{ length: numberOfPage },
(_, index) => index + 1,
);

return (
<ul className="pagination">
<li
className={classNames('page-item', {
disabled: currentPage === 1,
})}
>
<a
data-cy="prevLink"
className={classNames('page-link', {
disabled: currentPage === 1,
})}
onClick={e => {
if (currentPage !== 1) {
onPageChange(currentPage - 1);
}

e.preventDefault();
}}
aria-disabled={currentPage === 1}
>
«
</a>
</li>
{pagesList.map(i => (
<li
key={i}
className={classNames('page-item', { active: i === currentPage })}
>
<a
data-cy="pageLink"
className="page-link"
href={`#${i}`}
onClick={e => {
if (currentPage !== i) {
onPageChange(i);
}

e.preventDefault();
}}
>
{i}
</a>
</li>
))}
<li
className={classNames('page-item', {
disabled: currentPage === numberOfPage,
})}
>
<a
data-cy="nextLink"
className={classNames('page-link', {
disabled: currentPage === numberOfPage,
})}
onClick={e => {
if (currentPage !== numberOfPage) {
onPageChange(currentPage + 1);
}

e.preventDefault();
}}
aria-disabled={currentPage === numberOfPage}
>
»
</a>
</li>
</ul>
);
};