-
Notifications
You must be signed in to change notification settings - Fork 394
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
Clear w/l display on player page switch #3150
Conversation
…gating between player profiles
I think we can achieve this without changing to link elements. Note that when navigating player profile to player profile, the matches, heroes, and peers show a loader without a page refresh. We want to achieve the same thing for the W/L number. |
… between player profiles
i see, ok after digging deeper, i managed to find the cause of the issue, and a (probably) better solution. Updated the PR! Please help to take a look, thanks Howard! |
@@ -1,4 +1,5 @@ | |||
import React from 'react'; | |||
import { withRouter } from 'react-router-dom'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this file needs to be modified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, forgot to remove while testing some stuff, thanks for spotting
Thanks! This is definitely better--ideally we would only show a separate loading state for the win loss section since that tends to load a little slower than the profile info, but that's not required |
hmmm yeah, actually if you want to show a separate loading state for the win loss section, i could just remove the extra condition in the "PlayerHeader" component. But my initial thought was that it would be more UI-friendly to load the entire component together rather than seeing portions of it loading separately. What dyou think? |
I think we should show a loader only for the w/l data. If the name/picture etc. are available first there's no reason to block on it. |
ok done, have removed the extra condition |
Looks good! I guess we can leave for a future item improving the loader apperance (maybe a shimmer instead of a spinner) and left align it. |
Cause of issue:
Proposed changes:
fixes #3148