Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
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://Anvrey.github.io/react_pagination/) and add it to the PR description.
9 changes: 5 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"devDependencies": {
"@cypress/react18": "^2.0.1",
"@mate-academy/scripts": "^1.9.12",
"@mate-academy/scripts": "^2.1.3",
"@mate-academy/students-ts-config": "*",
"@mate-academy/stylelint-config": "*",
"@types/node": "^20.14.10",
Expand Down
131 changes: 37 additions & 94 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,110 +1,53 @@
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.

You aren't using React Router here. To satisfy the starred requirement (#13/#25/#26) import and use React Router hooks (e.g. useSearchParams or useLocation + useNavigate) so you can read ?page=...&perPage=... on load and later update the URL when state changes.

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 component currently only imports React. To meet the requirement that ?page=...&perPage=... is read on load and persisted, import and use React Router hooks (e.g., useSearchParams or useLocation + useNavigate) so you can read query params on initial render and update them when pagination changes.

import './App.css';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor: the props interface name VisiblevisibleItems is confusing. Consider renaming to ItemListProps or similar for clarity (not required for functionality, but improves readability).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rename the props interface VisiblevisibleItems to a clearer name like ItemListProps for readability (this is a non-blocking suggestion but improves clarity).

import { getNumbers } from './utils';
import { Pagination } from './components/Pagination';
import { ItemList } from './components/ItemList/ItemList';
import { PageInfo } from './components/PageInfo';
import { PerPageSelector } from './components/PerPageSelector/PerPageSelector';

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

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.

Defaulting currentPage to 1 inside Pagination is acceptable for a stateless component, but it only meets the requirement when the App passes a validated value coming from the URL. Ensure the App provides a validated currentPage from parsed query params so the default isn't relied on unexpectedly.

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.

The app initializes currentPage via useState(1) but does not read ?page=...&perPage=... from the URL on load. The task requires using React Router to apply URL parameters on page load (e.g. read query params and initialize state accordingly).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do not initialize currentPage with a fixed default here. Instead parse page from the URL on initial render, validate and clamp it to the range [1, totalPages], and use that validated value as the initial state.

const [perPage, setPerPage] = useState(5);
Comment on lines +13 to +14
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 and perPage with hard-coded values. Parse page and perPage from the URL on initial render, validate them (clamp page to [1, totalPages] and ensure perPage is one of 3,5,10,20), and use those validated values to initialize state (lazy initializer or effect). This implements checklist items #13, #14 and #25.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do not initialize perPage with a fixed default here. Parse perPage from the URL, ensure it is one of the allowed values (3, 5, 10, 20), and default to 5 only if the query value is missing/invalid.

const startIndex = (currentPage - 1) * perPage;
const lastIndex = Math.min(startIndex + perPage, items.length);
Comment on lines +15 to +16
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Computing startIndex/lastIndex before validating / clamping currentPage can produce incorrect slices. Make sure currentPage is validated against totalPages first, then compute startIndex and lastIndex from the validated values.

const visibleItems = items.slice(startIndex, lastIndex);

const handlePageChange = (page: number) => {
setCurrentPage(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.

handlePageChange updates component state but does not update the URL. Per the requirements page changes must be saved in the URL (e.g. ?page=2&perPage=5) using React Router navigation/history so the current page is reflected in the address bar.

};
Comment on lines +19 to +21
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 handlePageChange should avoid no-op updates and also update the URL. Add a guard like if (page !== currentPage) { setCurrentPage(page); /* update URL via navigate or setSearchParams */ }. This ensures onPageChange/App state only change when the page actually changes (checklist #6, #27, #28) and lets you keep ?page=...&perPage=... in sync (#26).

Comment on lines +19 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handlePageChange should guard against no-op updates (e.g., if (page === currentPage) return;) and also synchronize the new page to the URL (use setSearchParams or navigate to write ?page=...&perPage=...). Right now it only calls setCurrentPage, so URL persistence is missing.


const handlePerPageChange = (pages: number) => {
setPerPage(pages);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handlePerPageChange updates perPage and resets the page to 1 (good), but it does not update the URL. Per the starred requirement you must save ?page=1&perPage=<value> in the URL when per-page changes so values persist and apply on reload.

setCurrentPage(1);
};
Comment on lines +23 to +26
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 per-page changes you should validate the new value (only accept 3/5/10/20), reset page to 1, and update the URL query string via React Router so ?page=1&perPage=... stays in sync and persists across reloads (checklist #11, #12, #26). Right now you update state only; add URL update here.

Comment on lines +23 to +26
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handlePerPageChange needs to validate the incoming value (allow only 3,5,10,20), avoid redundant updates, reset page to 1, and persist ?page=1&perPage=... to the URL. Currently it updates state but does not validate or update the query string.


return (
<div className="container">
<h1>Items with Pagination</h1>

<p className="lead" data-cy="info">
Page 1 (items 1 - 5 of 42)
</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>
</select>
</div>
<PageInfo
currentPage={currentPage}
startIndex={startIndex}
lastIndex={lastIndex}
total={items.length}
/>

<label htmlFor="perPageSelector" className="col-form-label col">
items per page
</label>
</div>
<PerPageSelector
perPage={perPage}
onPerPageChange={handlePerPageChange}
/>

{/* 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>
<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>
</ul>
<Pagination
total={items.length}
perPage={perPage}
currentPage={currentPage}
onPageChange={handlePageChange}
Comment on lines +44 to +48
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 pass currentPage and perPage into Pagination, but because App doesn't read initial query params or write updates to the URL, deep links and reloads will not reflect the correct pagination state. Integrate React Router syncing so the values rendered come from the validated query params and updates modify the URL.

/>
<ItemList visibleItems={visibleItems} />
</div>
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 link click handler calls onPageChange(page) unconditionally. Per the requirements the callback should be triggered only if the page actually changed — add a guard such as if (page !== currentPage) onPageChange(page) to avoid firing for the current page.

);
};
Expand Down
15 changes: 15 additions & 0 deletions src/components/ItemList/ItemList.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React 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 task requires using React Router to save and apply query params (e.g. ?page=2&perPage=7). There are no react-router imports or usage in this file — you should parse page and perPage from the URL on load and keep the URL in sync when they change.

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 task requires reading and writing pagination state from/to the URL using React Router (useSearchParams or useLocation/useNavigate). This file currently has no react-router usage: import and use the appropriate hooks so you can parse ?page and ?perPage on load and update the query string when the user changes page or perPage (check checklist items #13, #14, #15, #16, #17, #26).

interface VisiblevisibleItems {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Interface name VisiblevisibleItems is confusing. Consider renaming to something clearer like ItemListProps to improve readability (this is a minor, non-blocking suggestion from the checklist).

visibleItems: string[];
}
export const ItemList: React.FC<VisiblevisibleItems> = ({ visibleItems }) => {
return (
<ul>
{visibleItems.map(item => (
<li data-cy="item" key={item}>
{item}
</li>
))}
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 top-level component should read page and perPage from the URL on first render (using React Router hooks), validate them (clamp page to [1, totalPages], ensure perPage ∈ {3,5,10,20}), and apply them to state so deep links work. Add that logic here before computing visible items.

</ul>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

currentPage is initialized to 1 but the spec requires applying ?page from the URL on load. Consider reading page from the query (via useLocation / URLSearchParams) and using it to initialize or set currentPage (validate/normalize the value).

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 initialize currentPage with a fixed default. Per the task you must read page from the URL on initial load and validate/clamp it (to the range [1, totalPages]). Use React Router (e.g., useSearchParams or useLocation+useNavigate) to read the query params instead of hard-coded defaults.

);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

perPage is initialized to 5 but must also be loaded from the URL if ?perPage is present. Parse perPage from the query string on load and ensure the selector reflects it.

Comment on lines +13 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

currentPage and perPage are initialized here with static defaults. Per checklist items #13/#25 you must instead initialize them from the URL query string on initial render, validating values (clamp page to [1, totalPages] and ensure perPage is one of 3,5,10,20). Use parsed & validated values to set initial state instead of always starting with 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.

You initialize perPage with a fixed default. The app must read perPage from the URL on load and validate it (ensure it's one of 3, 5, 10, 20). Replace the fixed default with a value parsed from the query string.

};
20 changes: 20 additions & 0 deletions src/components/PageInfo.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React 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 task requires using React Router to save ?page=2&perPage=7 in the URL and apply them on page load. There is no router usage or query param parsing here. Add React Router hooks (e.g. useLocation/useHistory in v5 or useSearchParams/useNavigate in v6) and parse URL query params to initialize currentPage and perPage on mount.

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 need to import React Router hooks (for example useSearchParams or useLocation + useNavigate) and use them to read and persist ?page=...&perPage=.... The app currently has no URL synchronization which is required by the task (checklist items #13, #25, #26).

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 component currently doesn't import or use any React Router hooks. The task requires reading ?page and ?perPage from the URL on load and persisting changes back to the URL. Import and use useSearchParams or useLocation + useNavigate in App to read/write the query string so pagination state survives reloads/deep links.


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Optional: the interface name VisiblevisibleItems in ItemList looks like a typo. Consider renaming it to something clearer like ItemListProps. This won't affect functionality but improves readability.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor: the props interface name VisiblevisibleItems is confusing. Consider renaming it to ItemListProps (or similar) for clarity — this doesn't affect functionality but improves readability.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor: the props interface name VisiblevisibleItems is confusing. Consider renaming it to ItemListProps (or similar) for readability. This is non-blocking but improves maintainability.

interface PageInfoProps {
currentPage: number;
startIndex: number;
lastIndex: number;
total: number;
}
export const PageInfo: React.FC<PageInfoProps> = ({
currentPage,
startIndex,
lastIndex,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pagination is implemented as a stateless component, which is good. However, ensure the top-level App is the single source of truth for currentPage/perPage initialized from the URL; otherwise the UI may not reflect query params on load even though Pagination has a default currentPage=1.

total,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do not hardcode the initial currentPage with useState(1). You must parse page from the query string on initial render, clamp it to the valid range [1, totalPages], and initialize state from that parsed value (fallback to 1 only if the query param is missing/invalid).

}) => {
Comment on lines +13 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

currentPage and perPage are initialized with fixed defaults here. Per requirements you must initialize these values from the URL query string on first render (parse page and perPage, validate them: clamp page to [1, totalPages] and ensure perPage is one of [3,5,10,20]) so the initial state reflects ?page=...&perPage=... (checklist items #13, #14, #25).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do not hardcode the initial perPage with useState(5). Parse perPage from the query string and ensure it's one of the allowed options [3, 5, 10, 20]. Use a validated value (or fallback to a default) when initializing state so the app respects ?perPage=... on load.

return (
<p className="lead" data-cy="info">
Page {currentPage} (items {startIndex + 1} - {lastIndex} of {total})
</p>
);
};
Comment on lines +19 to +20
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 page changes the App updates internal state but does not update the URL. According to the requirements the App should save ?page=...&perPage=... in the URL. Update this handler to push/replace query params to the URL (so bookmark/refresh preserves current pagination).

79 changes: 78 additions & 1 deletion src/components/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
@@ -1 +1,78 @@
export const Pagination = () => {};
import React 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 task requires using React Router to read ?page and ?perPage from the URL on load and keep them in sync. Import and use useSearchParams or useLocation/useNavigate here to parse query params on initial render and to update the URL when page/perPage change (see checklist items about URL sync).

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 need to read initial pagination state from the URL and persist it back when it changes. Import and use React Router hooks (e.g., useSearchParams or useLocation + useNavigate) here to parse ?page and ?perPage on first render and to update the query string when state changes. This is required by the task (synchronize pagination with the URL).

import cn from 'classnames';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor readability suggestion: rename the VisiblevisibleItems interface to something clearer like ItemListProps (or similar). This makes the component API clearer and was recommended in the checklist as a low-priority improvement.

interface PaginationProps {
total: number;
perPage: number;
currentPage?: number;
onPageChange: (page: number) => void;
}
export const Pagination: React.FC<PaginationProps> = ({
total,
perPage,
currentPage = 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.

Pagination is currently implemented as a stateless component which is good, but relying on its internal default currentPage = 1 (here) is not sufficient: the top-level App must supply validated currentPage/perPage parsed from the URL. Ensure URL parsing/validation happens at the top level so Pagination receives correct values.

onPageChange,
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 with a fixed 1. You must derive the initial page from the URL query string (validate/clamp to [1, totalPages]) and use that value to initialize state so the URL controls the initial pagination (checklist items #3, #14, #25).

}) => {
Comment on lines +13 to +14
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 component/app does not read ?page and ?perPage from the URL on load. The task requires using React Router to apply URL params on page load (e.g. ?page=2&perPage=7). Instead of initializing state with fixed values, read search params and initialize currentPage and perPage from them (with sensible fallbacks).

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 hardcode perPage to 5. Parse perPage from the URL on initial render (allowed values: 3, 5, 10, 20), validate it (fall back to 5 if invalid), and use that parsed value to initialize this state. The selector already provides these options but initial state must reflect the URL (checklist items #11, #14, #25).

Comment on lines +13 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid initializing currentPage and perPage with fixed defaults here. Instead parse page and perPage from the query string, validate them (clamp page between 1 and totalPages and ensure perPage is one of [3,5,10,20]), and use those validated values as initial state. The task requires applying query params on page load rather than always starting from fixed defaults.

const totalPages = Math.ceil(total / perPage);
const pages = [];

Comment on lines +15 to +17
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

startIndex/lastIndex/visibleItems are computed from the current state — make sure they are computed after you parse and validate page/perPage from the URL. If you don't clamp page to [1, totalPages], these indices can be out of expected range and break the displayed info/items.

for (let i = 1; i <= totalPages; i++) {
pages.push(i);
}
Comment on lines +19 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Guard against no-op page changes and synchronize the URL. handlePageChange should do if (page === currentPage) return;, then update state and write the new ?page=...&perPage=... to the URL (e.g., via useNavigate/setSearchParams). The task requires triggering onPageChange only when the page actually changes and persisting it to the URL.


Comment on lines +19 to +21
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 page changes (handlePageChange) the app only updates internal state. The requirement asks to save ?page=...&perPage=... in the URL using React Router. Update handlePageChange to push/update the search params in the URL (so the URL reflects the current page).

Comment on lines +19 to +21
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-level handler should avoid unnecessary updates: add a guard if (page === currentPage) return; before calling setCurrentPage(...). Also, when the page actually changes, update the URL query string (e.g. via navigate({ search: ... }) or setSearchParams) so ?page=...&perPage=... stays in sync (checklist items #6, #26, #27, #28, #33).

return (
<ul className="pagination">
<li className={cn('page-item', { disabled: currentPage === 1 })}>
<a
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 perPage changes (handlePerPageChange) you reset to the 1st page which is correct, but you also must update the URL (?page=1&perPage=...) so the state is reflected in the query string and persisted on reload. Use React Router history/navigation to update the query params.

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 perPage changes, only update state if the new value differs from the current one, reset page to 1, and update the URL query string accordingly. Also validate the new perPage is one of [3,5,10,20] before applying it. This keeps state and URL synchronized and satisfies checklist items about per-page selector and URL persistence (#11, #12, #24, #26).

Comment on lines +23 to +25
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Validate the incoming perPage, avoid redundant updates, reset the page to 1 and persist ?page=1&perPage=... to the URL. handlePerPageChange should check that pages is one of [3,5,10,20], and skip updating if the selected value equals current perPage. Per the requirements, after changing perPage you must always show the 1st page and update the query string.

data-cy="prevLink"
className="page-link"
href="#prev"
aria-disabled={currentPage === 1}
onClick={e => {
e.preventDefault();
if (currentPage > 1) {
onPageChange(currentPage - 1);
}
}}
>
«
</a>
</li>
{pages.map(page => (
<li
className={cn('page-item', { active: page === currentPage })}
key={page}
>
<a
data-cy="pageLink"
className="page-link"
href={`#${page}`}
onClick={e => {
e.preventDefault();
if (page !== currentPage) {
onPageChange(page);
}
}}
>
{page}
</a>
</li>
))}
<li className={cn('page-item', { disabled: currentPage === totalPages })}>
<a
data-cy="nextLink"
className="page-link"
href="#next"
aria-disabled={currentPage === totalPages}
onClick={e => {
e.preventDefault();
if (currentPage < totalPages) {
onPageChange(currentPage + 1);
}
}}
>
»
</a>
</li>
</ul>
);
};
32 changes: 32 additions & 0 deletions src/components/PerPageSelector/PerPageSelector.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import React 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 task requires using React Router to save and apply query params (e.g. ?page=2&perPage=7). This file doesn't use any router hooks or parse the URL. You should read page and perPage from the query string on load and initialize state accordingly (with safe fallbacks), and keep the URL in sync when values change.

interface PerPageSelectorProps {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor: the interface name VisiblevisibleItems in ItemList appears to be a typo and reduces readability. Consider renaming to e.g. ItemListProps (no functional impact but improves clarity).

perPage: number;
onPerPageChange: (perPage: number) => void;
}
export const PerPageSelector: React.FC<PerPageSelectorProps> = ({
perPage,
onPerPageChange,
}) => {
return (
<div className="form-group row">
<div className="col-3 col-sm-2 col-xl-1">
<select
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

currentPage is initialized with a fixed value (useState(1)) but the spec requires applying ?page from the URL on page load. Parse search params and initialize or update currentPage from the URL (validate the value and ensure it fits within available pages).

data-cy="perPageSelector"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

perPage is initialized with a fixed value (useState(5)) but must be read from ?perPage when present. On load, parse and apply perPage from the query string so the selector and pagination reflect the URL.

id="perPageSelector"
className="form-control"
value={perPage}
onChange={e => onPerPageChange(+e.target.value)}
Comment on lines +17 to +18
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 select and options are implemented correctly. Ensure the parent (App) validates that the perPage prop is one of the allowed values (3, 5, 10, 20) and resets to a valid default if not. If perPage is out of that set the select may have no matching option selected — validation/clamping should happen upstream per task requirements.

>
<option value="3">3</option>
<option value="5">5</option>
Comment on lines +19 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handlePageChange only updates component state. Per the requirements, page changes must be saved in the URL (e.g. ?page=2&perPage=5). Update this handler to push/replace query params via React Router so the URL reflects the current page.

<option value="10">10</option>
<option value="20">20</option>
</select>
</div>
Comment on lines +23 to +25
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handlePerPageChange resets currentPage to 1 which is correct, but it doesn't update the URL. When perPage changes you must update the query string (e.g. ?page=1&perPage=10) so the selection persists and will be applied on reload.


<label htmlFor="perPageSelector" className="col-form-label col">
items per page
</label>
</div>
);
};
Loading