odota / web

React web interface for the OpenDota platform
https://www.opendota.com
MIT License
1.07k stars 390 forks source link

Clear w/l display on player page switch #3150

Closed jeyren95 closed 4 months ago

jeyren95 commented 4 months ago

Cause of issue:

Proposed changes:

fixes #3148

howardchung commented 4 months ago

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.

jeyren95 commented 4 months ago

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!

howardchung commented 4 months ago

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

jeyren95 commented 4 months ago

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?

howardchung commented 4 months ago

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.

jeyren95 commented 4 months ago

ok done, have removed the extra condition

howardchung commented 4 months ago

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.