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

Implement pagination for rankings overlay #31189

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

SailorSnoW
Copy link

@SailorSnoW SailorSnoW commented Dec 19, 2024

Closes #31036.

Fully functional but may need to polish the way we implement this because I'm not sure adding the pagination in the Header is the way we want this, waiting for some reviews.
I've tried to visually replicate the website implementation, apart that we only show one page selector (on top) instead of two (bottom and top) as I think having both is kinda useless and we should just choose if we put it on top or bottom of the table.

About the available pages, as we can't fetch the potential total that the API would fetch, we put it to 200 available pages (like in the web version).

This PR also change the way the TestRankingsOverlay is loaded for tests Set-up to better start from a clean instance on each test.

CleanShot.2024-12-19.at.01.45.35.mp4

@SailorSnoW SailorSnoW marked this pull request as ready for review December 20, 2024 00:40
@peppy
Copy link
Member

peppy commented Dec 24, 2024

I'd probably match web and have the pagination at the top and bottom, if it's not too much trouble.

@SailorSnoW
Copy link
Author

I don’t think it would cause too much trouble, I just don’t really have an idea at the moment on how to place the second PageSelector below the Table, especially if using the Header for the first one. Is there already a way implemented in OnlineOverlay to easily position elements at the bottom/under the main Content? I don't remember seeing something like that actually or I miss something.

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

Successfully merging this pull request may close these issues.

Rankings tab only shows top 50 players
3 participants