Update `UserGridPanel` layout #22184

stanriders · 2023-01-13T18:41:43Z

Requires Update status icon design #22182
Requires Refactor LevelBadge to use LevelInfo #22241
Requires Add CombinedGroupBadge #22280

bdach · 2023-01-13T21:57:19Z

Test failures appear to be relevant.

stanriders · 2023-01-14T13:24:00Z

From going through card usages I've found that some API calls that populate user data seem to not have information required for the cards:

GetUsersRequest (used in currently playing dashboard and solo spectator screens) doesn't have PlayMode and Statistics/Level set
GetFriendsRequest (used in dashboard) doesn't have PlayMode

Should I create an issue on osu-web or just hide the parts that are not being populated (as I already do with the playmode)?

bdach · 2023-01-14T13:17:37Z

osu.Game/Users/UserPanel.cs

@@ -61,27 +61,31 @@ protected UserPanel(APIUser user)
        [Resolved]
        protected OsuColour Colours { get; private set; } = null!;

+        protected virtual bool ShouldCreateUserCoverBackground { get; set; } = true;


I have a real strained relationship with these sorts of properties. Half the time these work ok, half the time they turn out to be a horrible mistake N months later down the lane when another design comes that doesn't fit the overfitted abstraction that these provide.

It's probably fine to have it for now, but it's written weird/wrong right now. It should be either a get-only virtual property, or a get; init non-virtual one - having it settable at any point is kind of a lie since it only has an effect when read once in BDL.

If I were to change this any further, I'd probably go for something like a IEnumerable<Drawable> CreateBackground() method, that in this implementation creates the box and the cover, and only the box in the one that doesn't want the cover. That's slightly less specific than this current setup.

Sure, I'll go with the CreateBackground()

bdach · 2023-01-14T13:21:01Z

osu.Game/Users/UserListPanel.cs

@@ -28,7 +28,7 @@ public UserListPanel(APIUser user)
        [BackgroundDependencyLoader]
        private void load()
        {
-            Background.Width = 0.5f;
+            Background!.Width = 0.5f;


Yeah no please don't do this. Debug.Assert() is much preferred.

That said if you go with my other CreateBackground() suggestion then this all can be refactored out to not require any of these gymnastics. It will result in some code duplication but I think it's fine.

osu.Game/Users/UserGridPanel.cs

bdach · 2023-01-14T13:40:15Z

osu.Game/Users/UserGridPanel.cs

+
+        protected override bool OnHover(HoverEvent e)
+        {
+            hoverDim?.FadeTo(0.2f, 200);


You might wanna put an easing on these, it looks pretty bad right now. OutQuint is what most of everything uses.

bdach · 2023-01-14T13:44:36Z

GetUsersRequest (used in currently playing dashboard and solo spectator screens) doesn't have PlayMode and Statistics/Level set

GetFriendsRequest (used in dashboard) doesn't have PlayMode

I think web is well within its right to not return these. They are both ruleset-sensitive pieces of data and it is unclear for which ruleset data should be returned in both cases.

stanriders · 2023-01-14T13:55:33Z

I think web is well within its right to not return these. They are both ruleset-sensitive pieces of data and it is unclear for which ruleset data should be returned in both cases.

PlayMode is supposed to return ruleset which is set as default for the user, I believe its set for everyone.
Statistics is a more weird one as the level badge is a big piece of the design, but I agree - it's not clear what is even supposed to be returned

bdach · 2023-01-14T14:01:09Z

PlayMode is supposed to return ruleset which is set as default for the user, I believe its set for everyone.

I suppose it is, although I don't know if that makes sense either. Like even bots seem to have ruleset set, while I'd probably accept null as the most viable alternative there.

Dunno. Maybe worth asking the web team, but I feel like the endpoints in question are likely to be returning UserCompact rather than User to manage load.

Co-authored-by: Bartłomiej Dach <[email protected]>

peppy · 2023-01-14T17:48:36Z

I don't quite get this design. Feels like a regression from what we currently have.

Here's some initial feedback:

The level badge looks wrong proportionally. There's too much empty space or the font weight is too low or something

@arflyte

Dimming on hover feels back-to-front. No border makes it feels like the cards are hardly even responding to the mouse.

2023-01-15.02.46.27.mp4

Also the bottom status part of the card not changing on hover feels bad.

Corner radius mismatches feel bad

Colour lines feel unbalanced and hard to read

Overlap galore

I dunno. The balance is just off

peppy

Not looking at code, but design-wise this needs some further thought.

bdach · 2023-01-14T17:51:01Z

Dimming on hover feels back-to-front

For whatever that's worth I suggested that (out of thin air) because as usual there are no hover states on design. The thought process was that the beatmap listing is also doing a similar thing by dropping colours a shade darker on hover.

peppy · 2023-01-14T18:00:28Z

I have a feeling I made the same comment for the beatmap cards too. I guess as long as they match let's go with it, but it definitely doesn't feel good here. Maybe the dim needs to be increased. The transition time definitely needs to be faster (match beatmap card?), and it should affect the status area too.

stanriders · 2023-01-14T23:14:25Z

A lot of overlap/balance issues might be coming from the card width - designs keep them at 375, which most of the card usages in game have them way smaller (as well as incosistent). I can try updating widths to the design, at least to see if that helps.

I will update the hover state to be the same as beatmap cards

…riders/osu into usergridpanel-design-update

arflyte · 2023-01-17T05:43:26Z

Hmm, probably this needs to go back to the drawing board (without changing too much).

peppy · 2023-01-17T06:10:45Z

@arflyte for the corner radius issue, adding a small amount of padding around the avatar will likely fix it. for the level badge, making the edge (shape) border thicker and changing the text size based on how many digits to better fit will likely work well.

not sure about the colour lines. the height definitely feels weird no matter how long i look at it.

pull-request-size bot added the size/L label Jan 13, 2023

stanriders mentioned this pull request Jan 13, 2023

Update user card design #22130

Open

8 tasks

Update UserGridPanel layout

acb41e9

bdach added the type:cosmetic label Jan 13, 2023

stanriders and others added 4 commits January 14, 2023 01:00

Ignore null possibility check

a022e2c

Fix solo spectator screen crash

505013c

Don't draw ruleset icon when there's no PlayMode provided by API

fb52a62

Merge branch 'master' into usergridpanel-design-update

2ef61a4

bdach self-requested a review January 14, 2023 13:22

Using

97ecb66

bdach requested changes Jan 14, 2023

View reviewed changes

Update osu.Game/Users/UserGridPanel.cs

7ba6cb5

Co-authored-by: Bartłomiej Dach <[email protected]>

peppy self-requested a review January 14, 2023 17:38

Fix compilation error

1773095

peppy reviewed Jan 14, 2023

View reviewed changes

stanriders added 4 commits January 15, 2023 04:08

Review suggestions

17fead5

Merge branch 'usergridpanel-design-update' of https://github.com/stan…

1838b97

…riders/osu into usergridpanel-design-update

Merge branch 'master' into usergridpanel-design-update

e155bd2

Fix master merge

7d7ef67

peppy self-requested a review January 17, 2023 04:55

Merge branch 'master' into usergridpanel-design-update

29e77e1

stanriders marked this pull request as draft December 21, 2024 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `UserGridPanel` layout #22184

Update `UserGridPanel` layout #22184

stanriders commented Jan 13, 2023 •

edited

Loading

bdach commented Jan 13, 2023

stanriders commented Jan 14, 2023

bdach Jan 14, 2023

stanriders Jan 14, 2023

bdach Jan 14, 2023

bdach Jan 14, 2023

bdach commented Jan 14, 2023 •

edited

Loading

stanriders commented Jan 14, 2023

bdach commented Jan 14, 2023

peppy commented Jan 14, 2023

peppy left a comment

bdach commented Jan 14, 2023

peppy commented Jan 14, 2023

stanriders commented Jan 14, 2023

arflyte commented Jan 17, 2023

peppy commented Jan 17, 2023

Update UserGridPanel layout #22184

Are you sure you want to change the base?

Update UserGridPanel layout #22184

Conversation

stanriders commented Jan 13, 2023 • edited Loading

bdach commented Jan 13, 2023

stanriders commented Jan 14, 2023

bdach Jan 14, 2023

Choose a reason for hiding this comment

stanriders Jan 14, 2023

Choose a reason for hiding this comment

bdach Jan 14, 2023

Choose a reason for hiding this comment

bdach Jan 14, 2023

Choose a reason for hiding this comment

bdach commented Jan 14, 2023 • edited Loading

stanriders commented Jan 14, 2023

bdach commented Jan 14, 2023

peppy commented Jan 14, 2023

The level badge looks wrong proportionally. There's too much empty space or the font weight is too low or something

Dimming on hover feels back-to-front. No border makes it feels like the cards are hardly even responding to the mouse.

Corner radius mismatches feel bad

Colour lines feel unbalanced and hard to read

Overlap galore

peppy left a comment

Choose a reason for hiding this comment

bdach commented Jan 14, 2023

peppy commented Jan 14, 2023

stanriders commented Jan 14, 2023

arflyte commented Jan 17, 2023

peppy commented Jan 17, 2023

Update `UserGridPanel` layout #22184

Update `UserGridPanel` layout #22184

stanriders commented Jan 13, 2023 •

edited

Loading

bdach commented Jan 14, 2023 •

edited

Loading