Skip to content
Open

Develop #1420

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
101 changes: 29 additions & 72 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,36 @@
import React 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 [currentPage, setCurrentPage] = React.useState(1);
const [perPage, setPerPage] = React.useState(5);
Comment on lines +10 to +11

Choose a reason for hiding this comment

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

The task requires using React Router to manage the page and perPage values in the URL. Currently, the state is initialized with static values. You should read these values from the URL's query parameters on page load to properly apply them.

Comment on lines +10 to +11

Choose a reason for hiding this comment

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

The task requires synchronizing the page and perPage state with the URL using query parameters (e.g., ?page=2&perPage=5). Currently, you're using local component state with useState, which doesn't persist in the URL. Please use the useSearchParams hook from react-router-dom to read the initial state from the URL and to update it when the page or per-page value changes.

Comment on lines +10 to +11

Choose a reason for hiding this comment

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

According to the task requirements, the currentPage and perPage values should be managed through URL query parameters, not local component state. Please use the useSearchParams hook from react-router-dom to read these values from the URL on load and to update the URL when they change.


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 {(currentPage - 1) * perPage + 1} -{' '}
{Math.min(currentPage * perPage, 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={e => {
setPerPage(Number(e.target.value));
setCurrentPage(1);
}}
>
<option value="3">3</option>
<option value="5">5</option>
<option value="10">10</option>
Expand All @@ -33,77 +44,23 @@ export const App: React.FC = () => {
</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} // total number of items to paginate
perPage={perPage} // number of items per page
currentPage={currentPage} /* optional with 1 by default */
onPageChange={setCurrentPage}
/>

<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((currentPage - 1) * perPage, currentPage * perPage)
.map(item => {
return (
<li key={item} data-cy="item">
{item}
</li>
);
})}
</ul>
</div>
);
Expand Down
91 changes: 90 additions & 1 deletion src/components/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
@@ -1 +1,90 @@
export const Pagination = () => {};
import cn from 'classnames';
import { getNumbers } from '../../utils';

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

export const Pagination = ({
total,
perPage,
currentPage = 1,
onPageChange,
}: Props) => {
Comment on lines +11 to +16

Choose a reason for hiding this comment

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

Excellent work on making the currentPage prop optional in the type definition. To fully meet the requirement (currentPage={1} /* optional with 1 by default */), you also need to provide a default value of 1. You can easily set this up during prop destructuring, like currentPage = 1.

const pagesCount = Math.ceil(total / perPage);
const items = getNumbers(1, pagesCount);

const handlePrevClick = (event: React.MouseEvent<HTMLAnchorElement>) => {
event.preventDefault();
if (currentPage && currentPage > 1) {
onPageChange(currentPage - 1);
}
};

const handleNextClick = (event: React.MouseEvent<HTMLAnchorElement>) => {
event.preventDefault();
if (currentPage && currentPage < pagesCount) {
onPageChange(currentPage + 1);
}
};

const handlePageClick = (
event: React.MouseEvent<HTMLAnchorElement>,
pageNumber: number,
) => {
event.preventDefault();

if (pageNumber !== currentPage) {
onPageChange(pageNumber);
}
};

return (
<ul className="pagination">
<li className={cn('page-item', { disabled: currentPage === 1 })}>
<a
data-cy="prevLink"
className="page-link"
href="#prev"
aria-disabled={currentPage === 1}
onClick={handlePrevClick}
>
«
</a>
</li>
{items.map(n => (
<li
key={n}
className={cn('page-item', {
active: currentPage === n,
disabled: n === currentPage,

Choose a reason for hiding this comment

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

The requirements state that the current page's li element should have the active class, which you've correctly implemented. However, adding the disabled class here is not specified in the task and is redundant, as the click handler already prevents action on the current page.

Copy link
Author

Choose a reason for hiding this comment

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

Це ж додано, щоб візуально виділити не клікабельний стан

})}
>
<a
data-cy="pageLink"
className="page-link"
href={`#${n}`}
onClick={event => handlePageClick(event, n)}
>
{n}
</a>
</li>
))}

<li className={cn('page-item', { disabled: currentPage === pagesCount })}>
<a
data-cy="nextLink"
className="page-link"
href="#next"
aria-disabled={currentPage === pagesCount}
onClick={handleNextClick}
>
»
</a>
</li>
</ul>
);
};