Add season stats user profile display #11970
Conversation
| ready: true, | ||
| }, | ||
| style: { | ||
| classes: 'qtip qtip--daily-challenge', |
There was a problem hiding this comment.
could probably unify the modifier name somehow, though didn't really have any naming idea
|
|
||
| $rank = $season->userScores() | ||
| ->where('user_id', '<>', $score->user_id) | ||
| ->where('total_score', '>=', $score->total_score) |
| return \Cache::remember( | ||
| static::seasonCacheKey($user->getKey(), $season->getKey()), | ||
| 600, | ||
| function () use ($season, $user) { |
There was a problem hiding this comment.
moving this to its own function would be useful for testing
| return \Cache::forget(static::seasonCacheKey($userId, $seasonId)); | ||
| } | ||
|
|
||
| private static function seasonCacheKey(int $userId, int $seasonId): string |
There was a problem hiding this comment.
the functions are all using same (or similar) set of parameters so might as well make it a proper class with fields and stuff.
| </div> | ||
| <div className='season-stats-popup__threshold'> | ||
| {trans('users.show.season_stats.division_top_percentage', { | ||
| value: stats.division.threshold * 100, |
There was a problem hiding this comment.
use formatNumber of style percent.
| } | ||
|
|
||
| if (!isset($userDivision)) { | ||
| return null; |
There was a problem hiding this comment.
returning null means it won't be cached. Maybe return false and use null_if_false
| border-radius: @border-radius-large; | ||
| display: flex; | ||
| align-items: center; | ||
| padding: 5px 8px 5px 5px; |
There was a problem hiding this comment.
make it just 5px, add 5px gap, set __name margin to 0 3px, and remove the margin lefts
| // See the LICENCE file in the repository root for full licence text. | ||
|
|
||
| .season-stats { | ||
| @_img-height: 35px; |
There was a problem hiding this comment.
is this the division image actual height?
| // See the LICENCE file in the repository root for full licence text. | ||
|
|
||
| .season-stats-popup { | ||
| @_gap: 10px; |
There was a problem hiding this comment.
the variable doesn't really make sense... they're used differently in different places. might as well not use variable at all
| border-radius: @border-radius-small; | ||
| } | ||
|
|
||
| &__main { |
There was a problem hiding this comment.
I find the alignment weird. It's centered but the separator in the middle make it look like the separator should be exactly on the center except it's probably not.
Maybe have the ranking on the left as small as possible with auto auto 1fr (with actual line div so a simple 15px gap can be used to match the padding), and centered image section.
Or maybe even move the rank to be another row (but with larger value size) along with total score
| justify-content: center; | ||
| align-items: center; | ||
| padding-left: calc(@_gap * 2); | ||
| border-left: 2px solid hsl(var(--hsl-c1)); |
There was a problem hiding this comment.
maybe match the other separator color on the page
Part of #8736.
Decided to put it in the header after some playing around because it felt most fitting there, any further design improvement suggestions are more than welcome.
Also adds
colour_tiercolumn to the divisions table, for the accent line display.Preview