Skip to content
Open

develop #1478

Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

> Here is the [working version](https://mate-academy.github.io/react_pagination/)

You a given a list of items and markup for the `Pagination`. Implement the
You a given a list of items and markup for the `Pagination`. Implement the
`Pagination` as a stateless component to show only the items for a current page.

1. The `Pagination` should be used with the next props:
Expand Down Expand Up @@ -32,4 +32,4 @@ You a given a list of items and markup for the `Pagination`. Implement the
- Implement a solution following the [React task guideline](https://github.com/mate-academy/react_task-guideline#react-tasks-guideline).
- Use the [React TypeScript cheat sheet](https://mate-academy.github.io/fe-program/js/extra/react-typescript).
- Open one more terminal and run tests with `npm test` to ensure your solution is correct.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://<your_account>.github.io/react_pagination/) and add it to the PR description.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://fedechkova.github.io/react_pagination/) and add it to the PR description.
126 changes: 51 additions & 75 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,56 @@
import React from 'react';
import './App.css';
import { getNumbers } from './utils';
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 BrowserRouter is imported from react-router here; it must come from react-router-dom. Change to: import { BrowserRouter } from 'react-router-dom'; so the router works and useSearchParams/URL handling functions correctly.

import { Pagination } from './components/Pagination';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Importing Pagination with ./components/Pagination will only work if there's an index barrel file re-exporting the component. The actual file is ./components/Pagination/Pagination.tsx; either import that explicit path or add an index.ts to re-export Pagination to avoid a module-not-found error.

import { useSearchParams } from 'react-router';
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 useSearchParams hook is imported from react-router here, but it should be imported from react-router-dom. Change to: import { useSearchParams } from 'react-router-dom'; — otherwise the routing hook will be undefined and URL params won't be read/saved as required by the task.


// eslint-disable-next-line @typescript-eslint/no-unused-vars
const items = getNumbers(1, 42).map(n => `Item ${n}`);
const items: string[] = getNumbers(1, 42).map(n => `Item ${n}`);

const allowedPerPageValues = [3, 5, 10, 20];

export const App: React.FC = () => {
const [searchParams, setSearchParams] = useSearchParams();

const rawPage = Number(searchParams.get('page')) || 1;
const rawPerPage = Number(searchParams.get('perPage')) || 5;

const itemsPerPage = allowedPerPageValues.includes(rawPerPage)
? rawPerPage
: 5;

const totalPages = Math.ceil(items.length / itemsPerPage);
const currentPage = rawPage >= 1 && rawPage <= totalPages ? rawPage : 1;

const startIndex = (currentPage - 1) * itemsPerPage;
const endIndex = Math.min(startIndex + itemsPerPage, items.length);
const visibleItems = items.slice(startIndex, endIndex);

const updateParams = (page: number, perPage: number) => {
setSearchParams({
page: String(page),
perPage: String(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 ${startIndex + 1} - ${endIndex} 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={itemsPerPage}
onChange={(e: React.ChangeEvent<HTMLSelectElement>) => {
updateParams(1, Number(e.target.value));
}}
>
<option value="3">3</option>
<option value="5">5</option>
<option value="10">10</option>
Expand All @@ -32,78 +63,23 @@ 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
currentPage={currentPage}
total={items.length}
perPage={itemsPerPage}
onPageChange={(page: number) => {
if (page !== currentPage) {
updateParams(page, itemsPerPage);
}
}}
/>

<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 key={item} data-cy="item">
{item}
</li>
))}
</ul>
</div>
);
Expand Down
84 changes: 83 additions & 1 deletion src/components/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
@@ -1 +1,83 @@
export const Pagination = () => {};
import React from 'react';

type PaginationProps = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In index.tsx you import BrowserRouter from 'react-router'. Like the hook above, BrowserRouter should be imported from 'react-router-dom'. Update the import to import { BrowserRouter } from 'react-router-dom'; to ensure routing works.

currentPage?: number;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ensure the import path for the Pagination component is resolvable. You're importing from './components/Pagination' — that only works if there is an index barrel file exporting Pagination. If you don't have such a re-export, import directly from ./components/Pagination/Pagination or add an index file that re-exports the component.

total: number;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Importing useSearchParams from 'react-router' is incorrect. The hook is provided by 'react-router-dom' in typical React Router v6 usage. Change to import { useSearchParams } from 'react-router-dom'; so the hook is resolved at runtime.

perPage: number;
onPageChange: (page: number) => void;
};

export const Pagination: React.FC<PaginationProps> = ({
currentPage = 1,
total,
perPage,
onPageChange,
}) => {
const totalPages = Math.ceil(total / perPage);
const pages = Array.from({ length: totalPages }, (_, i) => i + 1);

const isFirstPage = currentPage === 1;
Comment on lines +17 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.

You restrict itemsPerPage to allowedPerPageValues. That means arbitrary perPage values in the URL (for example ?perPage=7) will be ignored and fall back to 5. If the task expects applying arbitrary perPage values from the URL, remove this filter; if you want to keep the selector limited to specific options, document/accept that URL values outside the allowed set will revert to default.

const isLastPage = currentPage === totalPages;

const handlePageChange = (page: number) => {
if (page !== currentPage) {
onPageChange(page);
}
};

return (
<ul className="pagination">
<li className={`page-item ${isFirstPage ? 'disabled' : ''}`}>
<a
data-cy="prevLink"
className="page-link"
href="#prev"
aria-disabled={isFirstPage}
onClick={e => {
e.preventDefault();

if (!isFirstPage) {
handlePageChange(currentPage - 1);
}
}}
>
«
</a>
</li>

{pages.map(page => (
<li
key={page}
className={`page-item ${currentPage === page ? 'active' : ''}`}
>
<button
type="button"
data-cy="pageLink"
className="page-link"
onClick={() => handlePageChange(page)}
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 page number links are rendered as <button> elements. The task states to keep the HTML structure; if the original markup expected anchors for page links, replacing them with <button> changes the structure and may break tests that assert the element type. Consider using <a> with appropriate href (and preventing default on click) to preserve the original structure if required.

>
{page}
</button>
</li>
))}

<li className={`page-item ${isLastPage ? 'disabled' : ''}`}>
<a
data-cy="nextLink"
className="page-link"
href="#next"
aria-disabled={isLastPage}
onClick={e => {
e.preventDefault();

if (!isLastPage) {
handlePageChange(currentPage + 1);
}
}}
>
»
</a>
</li>
</ul>
);
};
15 changes: 11 additions & 4 deletions src/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { createRoot } from 'react-dom/client';
import React from 'react';
import ReactDOM from 'react-dom/client';
import { BrowserRouter } from 'react-router';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Importing BrowserRouter from 'react-router' is wrong; it should be imported from 'react-router-dom'. Because of this the router wrapper won't be set up correctly and URL search params won't be managed as required.

import App from './App';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Importing useSearchParams from 'react-router' is incorrect. The hook comes from 'react-router-dom'. This prevents the app from using React Router search params and so the ?page and ?perPage URL behavior will not work.

import { App } from './App';

createRoot(document.getElementById('root') as HTMLElement).render(<App />);
ReactDOM.createRoot(document.getElementById('root') as HTMLElement).render(
<React.StrictMode>
<BrowserRouter>
<App />
</BrowserRouter>
</React.StrictMode>,
);
Loading