Skip to content

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
@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