solutioin by artembaranovsky1 · Pull Request #1423 · mate-academy/react_pagination

artembaranovsky1 · 2026-01-20T11:43:42Z

mateacademy-ai-mentor

Review Summary

Great job on the visual implementation of the pagination component! The core logic is mostly in place, but there are two critical requirements that need to be addressed before this can be approved.

First, the current page and perPage values are not stored in the URL. The task requires using React Router to manage this state with query parameters (e.g., ?page=2&perPage=5). This involves reading the parameters from the URL in App.tsx and updating the href attributes in Pagination.tsx to be actual links instead of hash fragments.

Second, the onPageChange function is called even when the page doesn't change, such as when clicking the active page or a disabled arrow. Please add conditional logic to your onClick handlers in Pagination.tsx to ensure onPageChange is only called when selecting a different page.

You're very close to a complete solution. 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-20T11:45:31Z

src/components/Pagination/Pagination.tsx

+        <a
+          data-cy="prevLink"
+          className="page-link"
+          href="#prev"


The task requires using React Router to manage state in the URL with query parameters (e.g., ?page=2&perPage=5). The href attributes should be updated to generate these URLs instead of using hash fragments like #prev. This applies to all navigation links in this component.

mateacademy-ai-mentor · 2026-01-20T11:45:31Z

src/components/Pagination/Pagination.tsx

+          className="page-link"
+          href="#prev"
+          aria-disabled={currentPage === 1}
+          onClick={() => onPageChange(currentPage - 1)}


While the link is visually disabled on the first page, the onClick handler can still be triggered. The requirement is that onPageChange should only be called if the page has changed. You should add a condition here to prevent calling onPageChange when on the first page.

mateacademy-ai-mentor · 2026-01-20T11:45:31Z

src/components/Pagination/Pagination.tsx

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


The requirements state that onPageChange should only be triggered if the page was changed. Currently, clicking the active page link still calls this function. Please add a condition to call onPageChange only when the selected page is different from the current one.

mateacademy-ai-mentor · 2026-01-20T11:45:31Z

src/components/Pagination/Pagination.tsx

+          className="page-link"
+          href="#next"
+          aria-disabled={currentPage === totalPages}
+          onClick={() => onPageChange(currentPage + 1)}


Similar to the 'previous' link, this onClick handler should be conditional. It should only call onPageChange if the current page is not the last page. This is to fulfill the requirement that the callback is only triggered when the page actually changes.

mateacademy-ai-mentor · 2026-01-20T11:45:31Z

src/App.tsx

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

 export const App: React.FC = () => {


The task requires using React Router to manage page and perPage in the URL query parameters. This component should read these parameters from the URL on initial load and update them whenever the page or items per page changes.

mateacademy-ai-mentor · 2026-01-20T11:45:31Z

src/App.tsx

+
+  const total: number = items.length;
+  const first = (currentPage - 1) * perPage + 1;
+  const last = Math.min(currentPage * perPage, 42);


It's better to use the total variable here instead of the hardcoded value 42. This makes the component more reusable and less prone to errors if the total number of items changes.

mateacademy-ai-mentor · 2026-01-20T11:45:31Z

src/App.tsx

+            className="form-control"
+            value={perPage}
+            onChange={option => {
+              setPerPage(option.target.value);


option.target.value is a string, but the perPage state is typed as a number. You should convert the value to a number (e.g., using Number()) before setting the state to avoid potential bugs in calculations.

mateacademy-ai-mentor

Review Summary

Great progress on this task! You've successfully addressed the previous feedback, and the solution is very close to complete. However, I'm requesting changes because the core requirement to manage pagination state via the URL is not yet fully implemented.

There are two main issues to address:

State is not loaded from the URL: In src/App.tsx, the currentPage and perPage states are initialized with hardcoded values. The task requires reading these values from the URL's query parameters on page load. You can use the useSearchParams hook from react-router-dom to get the initial values from the URL.
The "Next Page" link is broken: In src/components/Pagination/Pagination.tsx, the to prop for the » link uses single quotes (' ') for its template string. This prevents the variables from being inserted correctly. Please change the single quotes to backticks (`) to fix the link.

Once these two items are fixed, the task 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-20T19:20:57Z

src/App.tsx

+  const [perPage, setPerPage] = React.useState<number>(5);
+  const [currentPage, setCurrentPage] = React.useState<number>(1);


The task requires using React Router to save and load page and perPage from the URL query parameters. Currently, you are using local state with hardcoded initial values. You should use a hook like useSearchParams to get the initial values from the URL and also to update the URL when the page or items-per-page changes.

mateacademy-ai-mentor · 2026-01-20T19:20:57Z

src/components/Pagination/Pagination.tsx

+        <Link
+          data-cy="nextLink"
+          className="page-link"
+          to={'?page=${currentPage + 1}&perPage=${perPage}'}


This link for the next page won't work as expected due to a small typo. Template literals require backticks (`) instead of single quotes (' ') to correctly insert the variable values into the string.

mateacademy-ai-mentor

Review Summary

Excellent work on this iteration! I am approving your solution as you've successfully addressed all the feedback and met every requirement. The application now correctly uses URL search parameters as the single source of truth for state, which is a fantastic implementation.

I've left a couple of minor, non-critical suggestions in src/components/Pagination/Pagination.tsx regarding code cleanup, such as removing commented-out code and unused imports. These are just best practices to keep your code tidy.

Overall, this is a very strong submission. Well done!

✨ 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-20T20:28:57Z

src/components/Pagination/Pagination.tsx

@@ -1 +1,102 @@
-export const Pagination = () => {};
+// import { getNumbers } from '../../utils';


This import is commented out and appears to be unused. It's a good practice to remove unused code to keep the file clean.

mateacademy-ai-mentor · 2026-01-20T20:28:57Z

src/components/Pagination/Pagination.tsx

+  const arrNumber = [];
+
+  for (let i = 0; i < totalPages; i++) {
+    arrNumber.push(i + 1);
+  }


This for loop works perfectly for generating the page numbers. As a suggestion for a more modern and concise approach, you could achieve the same result in a single line using Array.from:

const arrNumber = Array.from({ length: totalPages }, (_, i) => i + 1);

This is just an alternative way to write it, not a required change.

mateacademy-ai-mentor · 2026-01-20T20:28:57Z

src/components/Pagination/Pagination.tsx

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


This block of code is commented out. To improve code readability and maintainability, it's best to remove any dead or commented-out code before finalizing your work.

solutioin

9d41411

mateacademy-ai-mentor suggested changes Jan 20, 2026

View reviewed changes

fix

ad34cc2

artembaranovsky1 requested a review from mateacademy-ai-mentor January 20, 2026 19:19

mateacademy-ai-mentor suggested changes Jan 20, 2026

View reviewed changes

fix

3288497

artembaranovsky1 requested a review from mateacademy-ai-mentor January 20, 2026 20:27

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

View reviewed changes

		const [perPage, setPerPage] = React.useState<number>(5);
		const [currentPage, setCurrentPage] = React.useState<number>(1);

		@@ -1 +1,102 @@
		export const Pagination = () => {};
		// import { getNumbers } from '../../utils';

Conversation

artembaranovsky1 commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

artembaranovsky1 commented Jan 20, 2026 •

edited

Loading