Implemented pagination

[AndreyKagaml]

DEMO LINK

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are a couple of critical behaviors that don’t yet match the task requirements. The Pagination component currently treats currentPage as required and does not default it to 1, and the click handler can still call onPageChange even when the user clicks the already active page; both violate the requirement that currentPage is optional with a default of 1 and that onPageChange is fired only when the page actually changes. Please update the props type to currentPage?: number and provide a default in the destructuring (e.g. const { currentPage = 1, ... } = props), and adjust the click handler to resolve the clicked anchor via event.target.closest('a'), compute a single targetPage, respect aria-disabled, and only call onPageChange if targetPage !== currentPage and is within range.

In App.tsx, to fully support the optional URL behavior, you should read page and perPage from useSearchParams() on initial load (or in a useEffect) and use them to initialize/sync currentPage and perPage, and also ensure the URL is updated immediately when perPage changes via the selector so the URL always reflects the current UI state. These changes are important for correct behavior when reloading or sharing URLs. Separately (non-blocking but recommended), in SelectPerPage.tsx remove the selected attribute from the <option> and instead use defaultValue or a controlled value on the <select>, and add an explicit type for the onChange event parameter like React.ChangeEvent<HTMLSelectElement> for better TypeScript safety. Overall your structure and logic are solid; fixing these points will bring the component fully in line with the checklist and make the pagination more robust and predictable.

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are still a few critical issues that break required behavior and could cause runtime errors. In src/components/Pagination/Pagination.tsx, the click handler uses event.target.closest('a') without checking for null; if the user clicks on whitespace or the <ul> area, closest('a') can be null and link.getAttribute(...) will throw, so you should early-return when no link is found. The same handler should be refactored to compute a single targetPage (handling prev, next, and numeric links together) and only call onPageChange if targetPage !== currentPage and targetPage is within 1..countPages, and also immediately bail out if link.getAttribute('aria-disabled') === 'true' to avoid firing on disabled controls. In src/App.tsx, currentPage is parsed directly from searchParams.get('page') without a safe fallback, which can yield NaN or 0 when the param is missing or invalid; instead, parse with a default and ensure you always fall back to 1. Also in App.tsx, perPage is hardcoded and not initialized from the perPage query param, and when the user changes the per-page selector you update React state but do not update the URL; both page and perPage must be read from the URL on load and written back to the URL whenever they change so reloads and shared links preserve the UI state. Once you address the null guard and target-page logic in Pagination, and fully synchronize currentPage and perPage with searchParams in App (including resetting page to 1 in both state and URL when perPage changes), your solution should satisfy the task checklist. Overall the structure and components are solid, so fixing these focused issues will complete the implementation nicely.

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

The onPageChange handler updates search params using the current perPage state. If perPage has just changed this may be stale (and as noted above, perPage changes currently don't update the URL). Ensure you keep searchParams in sync whenever currentPage or perPage changes — e.g. call setSearchParams({ page: String(page), perPage: String(perPage) }) from both page and per-page change handlers or centralize this in a useEffect that runs when either state changes.

[mateacademy-ai-mentor]

Review Summary

I am approving your solution as an educational override so you can keep progressing while you refine a few remaining details.

You did a solid job making Pagination stateless, wiring it to App, slicing items correctly in ItemsList, and keeping page/perPage in sync with the URL; these are the core behaviors the task requires. The main thing to improve is that SelectPerPage is currently uncontrolled with defaultValue="5", so it won’t visually reflect a perPage value loaded from the URL (e.g. ?perPage=10); instead, pass the current perPage from App into SelectPerPage and use value={String(perPage)} on the <select> to keep UI and URL in sync. In Pagination.tsx, consider computing a single targetPage for prev/next/number links and only calling onPageChange when targetPage !== currentPage and 1 <= targetPage <= countPages, which will prevent out-of-range page changes and centralize your validation logic. Also, for better type-safety, avoid casting the result of closest('a') directly; let it be nullable (e.g. const link = (event.target as HTMLElement).closest('a')) and early-return if it’s null before using it.

Overall, your structure, props design, and URL-sync approach show good understanding of React and routing; polishing these small details will make your pagination fully robust and production-ready.

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Footnotes

1. Rate AI review example ↩

[mateacademy-ai-mentor]

The SelectPerPage component is rendered without the current perPage value from App. Pass the perPage state into SelectPerPage and make its a controlled input so the visible option matches the URL-initialized value on load. Currently SelectPerPage uses defaultValue="5" which will show 5 even when the URL has ?perPage=10 (see SelectPerPage implementation) .

[mateacademy-ai-mentor]

The component's props should include the currently selected perPage so the select can be controlled and reflect URL-initialized state on load. Update the interface to accept e.g. perPage: number and destructure it in the component signature.

[mateacademy-ai-mentor]

Change the component signature to accept the perPage prop so you can use it as the select value (example: export const SelectPerPage = ({ resetPage, perPage }: Prop) => {). This ensures the select reflects the App's perPage state coming from the URL.

[mateacademy-ai-mentor]

Remove the uncontrolled defaultValue="5". Use a controlled value={String(perPage)} bound to the prop passed from App so the visible option matches the URL/state on load.

[mateacademy-ai-mentor]

Type-safety: avoid casting the result of closest('a') directly to HTMLElement. Instead let link be possibly null (HTMLElement | null), early-return if null, then proceed. The current as HTMLElement cast hides the nullable type even though you check for null — prefer const link = (event.target as HTMLElement).closest('a'); if (!link) return; and then treat link as an HTMLAnchorElement afterwards.

[mateacademy-ai-mentor]

Click handler logic: refactor to compute a single numeric targetPage for all link types (prev/next/page). You already early-return when aria-disabled === 'true' which is good — after that compute targetPage (prev -> currentPage-1, next -> currentPage+1, page link -> parsed number). Then call onPageChange(targetPage) only if targetPage !== currentPage and targetPage >= 1 && targetPage <= countPages. Currently prev/next branch calls onPageChange without an explicit range check, which can produce invalid page numbers when countPages is 0 or otherwise unexpected. This change matches the previous review requirements and prevents out-of-range updates.

[2pasha]

good job! 👏

here is good point from AI mentor about pagination from URL params. Pagination works good using URL params but visually our selector doesn't change

Also try to pass all tests.

And fix comments below ⬇️

[2pasha]

remove unused code over the project

[2pasha]

move to separate fn

[2pasha]

move to separate fn