added solution by juliasemchishin · Pull Request #1419 · mate-academy/react_pagination

juliasemchishin · 2026-01-15T14:01:22Z

https://juliasemchishin.github.io/react_pagination/

mateacademy-ai-mentor

Review Summary

Great job implementing the core pagination logic! The component correctly displays items and handles page navigation. However, a few changes are needed to fully meet the task requirements, primarily related to integrating React Router for state management.

Here's what needs to be addressed:

URL State Management: In src/App.tsx, the currentPage and perPage state should be managed via URL query parameters (e.g., ?page=2&perPage=10). You can use the useSearchParams hook from react-router-dom to read the initial state from the URL and to update it when the user changes the page or items per page.
Navigation Links: In src/components/Pagination/Pagination.tsx, the navigation elements should be React Router <Link> components instead of <a> tags. The to prop should be used to construct a URL that updates the search parameters correctly.
Event Handler Logic: The onPageChange callback in Pagination.tsx should not be called when the user clicks on the link for the currently active page. Please add a check to prevent this unnecessary call.

You've built a solid foundation, and with these changes, your solution will be complete. Keep up the great work!

✨ 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-01-15T14:03:31Z

src/components/Pagination/Pagination.tsx

+          <a
+            data-cy="pageLink"
+            className="page-link"
+            href={`#${page}`}


The task requires using React Router to manage the page state in the URL. Instead of using a plain <a> tag with an onClick handler and a hash href, you should use a component from React Router (like <Link>) and construct a to prop that updates the URL search parameters (e.g., ?page=...). This comment also applies to the previous and next links.

mateacademy-ai-mentor · 2026-01-15T14:03:31Z

src/components/Pagination/Pagination.tsx

+            data-cy="pageLink"
+            className="page-link"
+            href={`#${page}`}
+            onClick={() => onPageChange(page)}


According to the requirements, the onPageChange callback should only be triggered if the page was changed. Currently, it's called even when clicking the link for the active page. You should add a condition to prevent this.

mateacademy-ai-mentor · 2026-01-15T14:03:31Z

src/App.tsx

+  const [currentPage, setCurrentPage] = useState(1);
+  const [perPage, setPerPage] = useState(5);


According to the task requirements, you need to use React Router to manage state in the URL. The initial values for currentPage and perPage should be read from the URL's search parameters (e.g., ?page=2&perPage=10). When these values change, the URL should also be updated. Consider using the useSearchParams hook from react-router-dom to achieve this.

mateacademy-ai-mentor · 2026-01-15T14:03:31Z

src/App.tsx

-        <li data-cy="item">Item 3</li>
-        <li data-cy="item">Item 4</li>
-        <li data-cy="item">Item 5</li>
+        {items.slice(startIndex, calculatedEndIndex).map(item => (


While using calculatedEndIndex works here because slice handles out-of-bounds indices, you have already calculated the correct endIndex on line 20. It's best practice to use endIndex here for consistency and to make the code's intent clearer.

mateacademy-ai-mentor

Review Summary

Great job building the pagination UI and logic! The component works well with local state. However, this submission requires changes because a core requirement—managing state via URL parameters—has not been met.

Here are the two main things to fix:

URL State Management: In src/App.tsx, please replace the useState hooks for currentPage and perPage with the useSearchParams hook from react-router-dom. This will allow the page state to be read from and saved to the URL's query parameters (e.g., ?page=2&perPage=10).
Router Navigation: In src/components/Pagination/Pagination.tsx, all navigation elements should be <Link> components from react-router-dom instead of <a> tags. You'll need to use the to prop to construct the correct URL with the appropriate search parameters.

Once these changes are made, the application state will be correctly preserved in the URL. Keep up the great work!

✨ 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-01-15T14:44:17Z

src/App.tsx

 const items = getNumbers(1, 42).map(n => `Item ${n}`);

 export const App: React.FC = () => {
+  const [currentPage, setCurrentPage] = useState(1);


The currentPage state should not be managed by useState. According to the task requirements, it needs to be read from the URL query parameters (e.g., ?page=2) and updated via React Router. You can use the useSearchParams hook for this.

mateacademy-ai-mentor · 2026-01-15T14:44:17Z

src/App.tsx


 export const App: React.FC = () => {
+  const [currentPage, setCurrentPage] = useState(1);
+  const [perPage, setPerPage] = useState(5);


Similarly, perPage should also be managed via URL search parameters (e.g., ?perPage=10), not local state. This will allow the application state to be preserved in the URL.

mateacademy-ai-mentor · 2026-01-15T14:44:17Z

src/App.tsx

+  function onPageChange(page: number): void {
+    if (page !== currentPage) {
+      setCurrentPage(page);
+    }
+  }


When using URL-based state, this function's responsibility will change. Instead of calling setCurrentPage to update a local state variable, it should update the page query parameter in the URL using the setter function from useSearchParams.

mateacademy-ai-mentor · 2026-01-15T14:44:17Z

src/App.tsx

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


This handler should also be updated to modify the URL search parameters instead of calling setPerPage and setCurrentPage. When the user selects a new value, you should update the perPage parameter in the URL and reset the page parameter to 1.

mateacademy-ai-mentor · 2026-01-15T14:44:17Z

src/components/Pagination/Pagination.tsx

+        <a
+          data-cy="prevLink"
+          className="page-link"
+          href="#prev"
+          aria-disabled={currentPage === 1 ? 'true' : 'false'}
+          onClick={() => {
+            if (currentPage > 1) {
+              onPageChange(currentPage - 1);
+            }
+          }}
+        >
+          «
+        </a>


According to the task requirements, navigation should be handled by React Router. This <a> tag should be replaced with a <Link> component from react-router-dom. The href prop should be replaced with a to prop that constructs the correct URL with search parameters. For example: to={?page=${currentPage - 1}&perPage=${perPage}}. Consequently, the onClick handler would no longer be necessary.

mateacademy-ai-mentor · 2026-01-15T14:44:17Z

src/components/Pagination/Pagination.tsx

+          <a
+            data-cy="pageLink"
+            className="page-link"
+            href={`#${page}`}
+            onClick={() => {
+              if (page !== currentPage) {
+                onPageChange(page);
+              }
+            }}
+          >
+            {page}
+          </a>


Similar to the previous/next links, this should be a <Link> component. The to prop should point to a URL that includes the specific page number, like to={?page=${page}&perPage=${perPage}}. The onClick handler is not needed when using <Link> for navigation.

mateacademy-ai-mentor · 2026-01-15T14:44:17Z

src/components/Pagination/Pagination.tsx

+        <a
+          data-cy="nextLink"
+          className="page-link"
+          href="#next"
+          aria-disabled={
+            currentPage === pagesCountArray.length ? 'true' : 'false'
+          }
+          onClick={() => {
+            if (currentPage < pagesAmount) {
+              onPageChange(currentPage + 1);
+            }
+          }}
+        >
+          »
+        </a>


This link should also be a <Link> component from react-router-dom to ensure that clicking it updates the URL's query parameters correctly.

mateacademy-ai-mentor

Review Summary

I'm approving this submission as an educational override to help you progress, as there are a couple of important refinements still needed. You've done a fantastic job switching to <Link> components for navigation!

The main area for improvement is in src/App.tsx. It still uses useState to manage the page state, which creates a separate source of truth from the URL. To fix this and resolve issues with browser navigation, please replace the useState hooks for page and perPage with the useSearchParams hook.

Additionally, since the <Link> components now handle navigation, the onPageChange prop and the onClick handlers in src/components/Pagination/Pagination.tsx are redundant and can be removed.

You're very close to the ideal solution. Keep up the great work!

✨ 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-01-15T20:47:42Z

src/App.tsx

+  const location = useLocation();
+  const navigate = useNavigate();
+  const params = new URLSearchParams(location.search);
+  const pageFromUrl = Number(params.get('page')) || 1;
+  const perPageFromUrl = Number(params.get('perPage')) || 5;
+
+  const [currentPage, setCurrentPage] = useState(pageFromUrl);
+  const [perPage, setPerPage] = useState(perPageFromUrl);


This block of code for managing URL parameters and state can be greatly simplified by using the useSearchParams hook, as mentioned in the previous review. Using useState here creates a separate source of truth from the URL, which causes bugs when using browser navigation buttons.

Please replace this manual implementation (useLocation, useNavigate, useState) with useSearchParams to manage the page and perPage values directly from the URL.

mateacademy-ai-mentor · 2026-01-15T20:47:42Z

src/components/Pagination/Pagination.tsx

+          onClick={() => {
+            if (currentPage > 1) {
+              onPageChange(currentPage - 1);
+            }
+          }}


This onClick handler is redundant. The <Link> component already handles navigation to the URL specified in the to prop. State should be updated in the parent component in response to this URL change, not through a separate callback mechanism like onPageChange.

mateacademy-ai-mentor · 2026-01-15T20:47:42Z

src/components/Pagination/Pagination.tsx

+            onClick={() => {
+              if (page !== currentPage) {
+                onPageChange(page);
+              }
+            }}


Similar to the other navigation links, this onClick handler duplicates the navigation logic that is already handled by the <Link> component's to prop. Relying solely on the URL change for state updates simplifies the data flow.

mateacademy-ai-mentor · 2026-01-15T20:47:42Z

src/components/Pagination/Pagination.tsx

+          onClick={() => {
+            if (currentPage < pagesAmount) {
+              onPageChange(currentPage + 1);
+            }
+          }}


This onClick handler is also redundant. The <Link> component's to prop is sufficient for navigation, and the parent component should react to the resulting URL change.

added solution

a9cecd0

mateacademy-ai-mentor suggested changes Jan 15, 2026

View reviewed changes

added solution

379cd72

juliasemchishin requested a review from mateacademy-ai-mentor January 15, 2026 14:42

mateacademy-ai-mentor suggested changes Jan 15, 2026

View reviewed changes

added sol

14426a4

juliasemchishin requested a review from mateacademy-ai-mentor January 15, 2026 20:45

mateacademy-ai-mentor approved these changes Jan 15, 2026

View reviewed changes

added fix

36602db

juliasemchishin requested a review from mateacademy-ai-mentor January 15, 2026 21:27

added fix

1c482d4

		const [currentPage, setCurrentPage] = useState(1);
		const [perPage, setPerPage] = useState(5);

Conversation

juliasemchishin commented Jan 15, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants