add task solution by maksymkolomiiets2-svg · Pull Request #1436 · mate-academy/react_pagination

maksymkolomiiets2-svg · 2026-02-14T19:50:05Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because a core requirement of the task—syncing pagination state with the URL via React Router—is not implemented yet, which means the feature set is incomplete. In src/App.tsx, currentPage and amount (per-page) are initialized with hardcoded values instead of being derived from ?page=...&perPage=... query parameters, so reloading or sharing the URL does not restore pagination state as required. Additionally, when the per-page selector changes and when onPageChange is triggered, you only update local state and do not update the URL’s query params, so the address bar never reflects the current page or per-page value. To fix this, you should import the appropriate React Router hooks (for example, useSearchParams in react-router-dom v6, or useLocation/useHistory in v5), read page and perPage once on mount to initialize state (with sensible fallbacks), and update these params in the URL whenever currentPage or amount changes. Once that synchronization is in place, your otherwise solid pagination UI will fully meet the task requirements. Overall, your Pagination component and UI behavior are well done; you just need to complete the router-based URL persistence to finish this task correctly.

✨ 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.

mateacademy-ai-mentor · 2026-02-14T19:52:59Z

src/App.tsx

@@ -1,25 +1,39 @@
-import React from 'react';
+import React, { useState } from 'react';


The task requires using React Router to save and read ?page=...&perPage=.... Currently there is no import/use of router hooks — add imports (e.g. useSearchParams from react-router-dom for v6 or useLocation/useHistory for v5) and use them to sync URL and state.

mateacademy-ai-mentor · 2026-02-14T19:52:59Z

src/App.tsx

+  const [amount, setAmount] = useState(5);
+  const [currentPage, setCurrentPage] = useState(1);


Initial state for amount and currentPage is hardcoded to 5 and 1. To satisfy the requirement to apply URL params on load, derive these initial values from the query string (or read them in a useEffect on mount) and fall back to defaults. Consider parsing page and perPage from the URL and passing them to useState initializers.

mateacademy-ai-mentor · 2026-02-14T19:52:59Z

src/App.tsx

+            onChange={e => {
+              setAmount(Number(e.target.value));
+              setCurrentPage(1);
+            }}


When the per-page selector changes you reset state (setAmount and setCurrentPage(1)) which is correct, but you also must update the URL query params (set page=1&perPage=<value>). Update this onChange handler to push the new query params to the router so the URL and state stay in sync (per task requirement).

mateacademy-ai-mentor · 2026-02-14T19:52:59Z

src/App.tsx

+        onPageChange={newPage => {
+          setCurrentPage(newPage);
+        }}


The onPageChange handler updates local currentPage only. Per requirements, also persist the new page in the URL (update page query param) — otherwise pagination state won't be reflected in the address bar and won't be restored on reload.

mateacademy-ai-mentor

Review Summary

I am requesting changes because a core requirement of the task—synchronizing pagination state with the URL using React Router—is not yet implemented, which means the feature set is incomplete. In src/App.tsx, amount (per-page) and currentPage are initialized with hardcoded values (5 and 1) instead of being derived from ?page and ?perPage query parameters on initial load, so reloading or sharing a link does not restore the correct state. You should use useSearchParams (or similar) to read page and perPage on mount, parse them as integers, validate them (e.g., valid number, within allowed options, page clamped to [1, totalPages]), and fall back to sensible defaults when invalid.

Additionally, when the user changes the per-page <select> you currently only update local state and reset to page 1; you must also write the new values to the URL, e.g. setSearchParams({ page: '1', perPage: String(newAmount) }), so the address bar reflects ?page=1&perPage=<selected>. Similarly, in your onPageChange handler you update currentPage locally but do not update the page query param—this must be synchronized as well (for example: setSearchParams({ page: String(newPage), perPage: String(amount) })). The Pagination UI itself is well implemented and your stateless Pagination component meets the markup and behavior requirements, so once you add this URL sync logic in App.tsx, your solution will satisfy the missing checklist items. Overall, your structure is solid; focus on wiring the state to React Router so pagination state is fully shareable and restorable via the URL.

✨ 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.

mateacademy-ai-mentor · 2026-02-14T22:29:20Z

src/App.tsx

+  const [amount, setAmount] = useState(5);
+  const [currentPage, setCurrentPage] = useState(1);


You initialize amount and currentPage with hardcoded values here. The task requires reading page and perPage from the URL query parameters on page load (e.g. ?page=2&perPage=7). Use React Router (for example useSearchParams from react-router-dom v6) to parse these params on mount, validate/parse them to integers with sensible fallbacks, and use them as the initial values instead of hardcoded 5 and 1. See the task description requirement to apply URL params on load.

mateacademy-ai-mentor · 2026-02-14T22:29:20Z

src/App.tsx

+            onChange={e => {
+              setAmount(Number(e.target.value));
+              setCurrentPage(1);
+            }}


When the per-page <select> changes you update local state and reset to page 1, but you do not update the URL. After calling setAmount(...) and setCurrentPage(1), also update the router search params so the address bar becomes ?page=1&perPage=<selected>. This is required by the checklist (sync perPage to URL).

mateacademy-ai-mentor · 2026-02-14T22:29:20Z

src/App.tsx

+        onPageChange={newPage => {
+          setCurrentPage(newPage);
+        }}


The onPageChange handler updates currentPage locally but does not update the URL query parameter page. To meet the task requirements, whenever the page changes (via page links or prev/next), update the search params (e.g. setSearchParams({ page: String(newPage), perPage: String(amount) })) so the address bar reflects the current page. Also ensure any search-param change from the URL (if you support it) updates the component state.

mateacademy-ai-mentor

Review Summary

I am approving your solution as part of an educational override so you can keep progressing while you learn and refine your React Router skills. Your Pagination component is very well done: it’s stateless, uses the correct props, renders all pages, handles prev/next correctly, and applies the required li.active, li.disabled, and data-cy attributes. In App.tsx, you correctly render the info text and per-page selector and wire the Pagination component so that page changes and per-page changes update local state.

The main gap is URL synchronization: setSearchParams is called around line 55 but never imported or initialized, which will cause a runtime error; you need to import useSearchParams from react-router-dom and call const [searchParams, setSearchParams] = useSearchParams() near the top of App.tsx. Also, amount and currentPage are initialized with hard-coded values instead of reading ?page and ?perPage from the URL—parse them from searchParams, validate perPage against the allowed options (3, 5, 10, 20), and clamp page into [1, totalPages]. When the per-page <select> changes, you correctly reset to page 1 in state, but you also need to write the new values to the URL, e.g. setSearchParams({ page: '1', perPage: String(newAmount) }), and in your onPageChange handler, reuse the current perPage from state or searchParams so it doesn’t get out of sync. Aside from these URL-related fixes, your structure and logic are solid, and with these adjustments you’ll fully meet the routing and persistence requirements—nice work so far, keep going!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-14T22:37:57Z

src/App.tsx

@@ -1,25 +1,38 @@
-import React from 'react';
+import React, { useState } from 'react';


You call setSearchParams later but never import or initialize it. Import useSearchParams from react-router-dom and call const [searchParams, setSearchParams] = useSearchParams() (and import useEffect if you plan to apply params on mount). Without this, the call at line 55 will cause a runtime error.

mateacademy-ai-mentor · 2026-02-14T22:37:57Z

src/App.tsx

+  const [amount, setAmount] = useState(5);
+  const [currentPage, setCurrentPage] = useState(1);


State variables amount and currentPage are initialized with hard-coded values. The task requires applying ?page and ?perPage from the URL on load. Use useSearchParams() to read and parse page and perPage (as integers), validate perPage against allowed options (3, 5, 10, 20), and clamp page into [1, totalPages]. Use a lazy initializer or a useEffect to set these on mount.

mateacademy-ai-mentor · 2026-02-14T22:37:57Z

src/App.tsx

+            onChange={e => {
+              setAmount(Number(e.target.value));
+              setCurrentPage(1);
+            }}


When the per-page <select> changes you reset local state to page 1 but do not update the URL. Per the requirements you must also write query params (e.g. setSearchParams({ page: '1', perPage: String(newAmount) })) so the address bar reflects the new state and the page is shareable/restorable.

mateacademy-ai-mentor · 2026-02-14T22:37:57Z

src/App.tsx

+        onPageChange={newPage => {
+        setCurrentPage(newPage);
+        setSearchParams({ page: String(newPage), perPage: String(amount) });


In the onPageChange handler you call setSearchParams({ page: String(newPage), perPage: String(amount) }) but setSearchParams isn't defined in this file. Initialize setSearchParams via useSearchParams. Also ensure you preserve the current perPage when updating only page (or derive perPage from searchParams to avoid stale closures).

add task solution

90dfa8f

mateacademy-ai-mentor suggested changes Feb 14, 2026

View reviewed changes

add task solution

0e19e8c

maksymkolomiiets2-svg requested a review from mateacademy-ai-mentor February 14, 2026 22:26

mateacademy-ai-mentor suggested changes Feb 14, 2026

View reviewed changes

add task solution

98e6ae3

maksymkolomiiets2-svg requested a review from mateacademy-ai-mentor February 14, 2026 22:35

mateacademy-ai-mentor approved these changes Feb 14, 2026

View reviewed changes

		@@ -1,25 +1,39 @@
		import React from 'react';
		import React, { useState } from 'react';

		const [amount, setAmount] = useState(5);
		const [currentPage, setCurrentPage] = useState(1);

		@@ -1,25 +1,38 @@
		import React from 'react';
		import React, { useState } from 'react';

Comments

Conversation

maksymkolomiiets2-svg commented Feb 14, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants