Skip to content

add solution#1518

Open
natayakovleva wants to merge 1 commit into
mate-academy:masterfrom
natayakovleva:develop
Open

add solution#1518
natayakovleva wants to merge 1 commit into
mate-academy:masterfrom
natayakovleva:develop

Conversation

@natayakovleva

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it correctly implements all core pagination requirements and the app behaves as specified. Pagination is fully stateless, driven only by total, perPage, currentPage, and onPageChange, while App holds the state, slices items correctly, and passes the right props. The page list is computed properly, the active page is highlighted with li.active, and onPageChange is only fired when the page actually changes, avoiding unnecessary updates. The previous/next controls work correctly, including applying li.disabled and aria-disabled on the first and last pages, and the info text and data-cy attributes match the required format. Changing perPage via the selector (3, 5, 10, 20) updates the state and resets the current page to 1 as required.

The only notable omission is the optional React Router integration to sync page and perPage with the URL query string, but this is explicitly marked as optional and does not block task completion. If you want to improve your solution further, you could add router-based URL syncing so that pagination state survives reloads and can be shared via links. Overall, this is a solid implementation that meets the functional requirements—well done.


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

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants