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

Fix Storm Dashboard for Other Users #608

Merged
merged 3 commits into from
Mar 28, 2024

Conversation

HaonRekcef
Copy link
Contributor

This PR resolves the issue of displaying other users storm highscores. At the moment users would either be directed to their own highscore screen or encounter an error if not logged in.
Now it should correctly display the highscores of other users. Also, I have included the user's name in the screen title, as it's now possible to view highscores of other users.

Preview

image

I can not test it for iOS but I copied the code from perf_stats_screen.

One question remaining is whether we really should implement caching for the storm dashboard. Currently, if a user views their history, plays, beats their daily highscore and then revisits their history, the updated highscore isn't displayed.

@veloce
Copy link
Contributor

veloce commented Mar 26, 2024

So you're right we should remove caching to ensure it's always up to date.

I'm not sure about player name in title though. Because sometimes it is too long and cannot be displayed entirely. We could maybe show player's name as an intermediate level title in body, only for other players screens.

@HaonRekcef
Copy link
Contributor Author

True, sometimes the name gets cut off. But this also happens for the other stats screen, probably we should fix this on all of them?
Should I remove the caching for all users, one option would also be to just disable it for the logged in user.

@veloce
Copy link
Contributor

veloce commented Mar 26, 2024

Just for the logged in user makes sense yes but normally it is preferable not to cache a provider with parameters to avoid memory leaks, so you can disable it completely.

And yes we should also remove user name from perf stats screen title bar.

@HaonRekcef
Copy link
Contributor Author

Okay, I removed the caching. However, I haven't found a good way to position the username somewhere other than in the title bar. Designing isn't exactly my strong suit... 🙃

@veloce
Copy link
Contributor

veloce commented Mar 26, 2024

Yeah I see that is not obvious to make it look good.

Perhaps we can just not display the player's name for now. This screen has to be accessed from the player profile so you'll always know what you're looking at.

@HaonRekcef
Copy link
Contributor Author

I removed the names from the screens, so no more names that are cut off.

@veloce veloce merged commit 8691ecc into lichess-org:main Mar 28, 2024
3 checks passed
@HaonRekcef HaonRekcef deleted the fix-puzzle-storm-dashboard branch March 28, 2024 18:51
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