Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Search page] : Pagination #89

Merged
merged 11 commits into from
Jul 1, 2024
Merged

[Search page] : Pagination #89

merged 11 commits into from
Jul 1, 2024

Conversation

cmoinier
Copy link
Contributor

Description

This PR adds a pagination component to the search page.

To test

With the local dump, you will need to decrease the pageSize in SearchResultsComponent to 2-3 in order to see the pagination work properly. It is set to 18 because the ticket asked for a display of 3x6 cards.

@cmoinier cmoinier force-pushed the paginate-search-page branch from 6afb774 to 7606b1d Compare June 18, 2024 15:02
Copy link
Contributor

@Angi-Kinas Angi-Kinas left a comment

Choose a reason for hiding this comment

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

Thank you @cmoinier for implementing the pagination, it looks really cool! :)
I just see a small improvement in the the font-weight of the "previous" and "next" button and their respective icons.
The rest would be a nice to have :-)

@Angi-Kinas
Copy link
Contributor

@cmoinier thank you for the adaptions!
Looking at the mockups, I just realised one more thing. When being on page 1, there is no "previous" button and I guess for the "next" button it is the same logic, when you are on the last page.
I'm not sure if it is really important, since the buttons are disabled, but it would be closer to the mockup. I realized this, when having only one page:
image

Also something I discovered (by mistake). If you only have one page, but in the url you use _page=2, it goes to the next empty page. Is that what we expect?

Copy link
Contributor

@Angi-Kinas Angi-Kinas left a comment

Choose a reason for hiding this comment

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

Thank you for implementing my suggestions @cmoinier
LGTM 👍

@cmoinier cmoinier merged commit 6dc3151 into main Jul 1, 2024
7 checks passed
@cmoinier cmoinier deleted the paginate-search-page branch July 1, 2024 13:12
@cmoinier cmoinier restored the paginate-search-page branch July 11, 2024 10:56
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