Extract score and statistics components from BeatmapLeaderboardScore#37056
Extract score and statistics components from BeatmapLeaderboardScore#37056LiquidPL wants to merge 2 commits intoppy:masterfrom
BeatmapLeaderboardScore#37056Conversation
To be (re)used on the playlists leaderboard score component. The current playlists score shows three statistics (acc, playcount, completed maps), which is an amount that wouldn't fit on the non-full display mode where the statistics are stacked on top of each other. To avoid this, I've used the space normally used for mods on the beatmap score for one of the statistics. With that in mind, this commit also includes some minor changes to the statistic componennt to allow the label and value to be laid out horizontally.
bdach
left a comment
bdach
left a comment
There was a problem hiding this comment.
Part of me wonders whether this isn't premature / overeager code sharing and whether it wouldn't be better to start with a full 100% copy pasta approach to begin with. Especially so that the end result of the refactor is not yet in sight even as a proof of concept. That said at least this is appearing to go the composition route rather than conjuring abstract entities so I'm more partial to just nod along.
That said, visually, something's off with the horizontal variant of the statistic display. The baselines are all out of alignment.
UseFullGlyphHeight = false kinda works but I'm not sure that's a reliable fix because it will stop working as soon as the label uses any characters which go below glyph baseline.
| // We don't want the value setting the horizontal size, since it leads to wonky accuracy container length, | ||
| // since the accuracy is sometimes longer than its name. |
There was a problem hiding this comment.
| // We don't want the value setting the horizontal size, since it leads to wonky accuracy container length, | |
| // since the accuracy is sometimes longer than its name. | |
| // We don't want the value setting the horizontal size, since it leads to wonky overall length, | |
| // since the value is sometimes longer than its label. |
Well this is frustrating. The drawables themselves are both bottom aligned, so I guess this is because of some rendering issues in Although I don't think we'd have issues with stuff going below the baseline if the label is always upper cased. |
I've been working on one last night, alongside another work-in-progress PR that's supposed to extract the profile picture, user, and date into another component. 
(don't mind the score text not being right, haven't plugged in that properly yet) |
Other than throwing arbitrary paddings at it probably not.
Wouldn't be sure about that, think localisation. Languages like Vietnamese are especially egregious with this with the amount of accent marks, but something like French could break just as well ( |
Also I gotta say I'm not super sold on this. The "completed beatmaps" text being under the score doesn't look super great to me. I'd rather see it next to the other stats. That said that is a @peppy call ultimately. |
|
Completed beatmap looks weird (the number isn't the right size for the heading). Can agree with @bdach. Maybe change "plays" to "attempts" and "completed beatmaps" to "completed", or similarly short terms. |
What I'm the most worried about here is that we're not going to fit three statistics in a single row when stacked vertically in the more narrow configurations. I tried placing one of the stats separately from the two stacked ones and it just feels awkward (though it might just be me): 
|
|
Either way I'll reduce this PR to extracting just the statistic component I think. |
|
Could just completely switch directions into something like this? 
|
|
Superseded by #37069. |
3 participants
To be (re)used on the playlists leaderboard score component.
The current playlists score shows three statistics (acc, playcount, completed maps), which is an amount that wouldn't fit on the non-full display mode, especially when the statistics are stacked on top of each other.
To avoid this, I've used the space normally used for mods on the beatmap score for one of the statistics. With that in mind, this commit also includes some minor changes to the statistic componennt to allow the label and value to be laid out horizontally.
BeatmapLeaderboardScoreis unchanged:Here's how the
LeaderboardScoreDisplaylooks with a statistic under the score:And here's the same component haphazardly inserted into
BeatmapLeaderboardScoresince I don't have the playlists component ready yet:The final one will probably need the padding between the score section and the stats section to be larger, but that's a thing for later.