ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.24k stars 2.27k forks source link

Implement "form" check box control #29712

Closed bdach closed 1 month ago

bdach commented 1 month ago

Continuation of https://github.com/ppy/osu/issues/29643.

https://github.com/user-attachments/assets/ec47f608-3787-4a78-a9d7-c98d1f4664d6

peppy commented 1 month ago

A bit concerned that the checkbox is so far away from the actual text (rather than being next to it on the left). One would hope these would only be used in fixed-width displays, but I get the feeling that's not the case?

Like in the video when you're clicking to toggle my brain was very confused as to what was going on:

bdach commented 1 month ago

One would hope these would only be used in fixed-width displays

Not sure what you mean by "fixed width" in this context. I generally don't intend this component to be used or even usable in contexts where it's allowed to autosize width wise. The design only makes sense in vertically flowing forms where every single field is of equal width.

peppy commented 1 month ago

Not sure what you mean by "fixed width" in this context

I just mean we probably want to avoid scenarios where this distance is more than 50px or so

2024-09-12 01 47 06@2x

bdach commented 1 month ago

Hmm okay... I'm not super sure I can guarantee that.

I'll consider options. Maybe the nub can be moved to the left of text instead.

peppy commented 1 month ago

Also the text is still completely weird to me. Like there's a checkbox title, then a customised yes/no line which – at least in the two provided examples – adds nothing but confusion to my brain. Like, the checkbox is there to imply state, so adding text just throws me off. I've never seen this done anywhere and I'm not sure why we're doing it here.

It would also mean more localisation work for no added benefit.

bdach commented 1 month ago

The customised yes/no thing is something I pulled from a design sketch in https://www.figma.com/file/IjrU0u7MFdgmxDofqTlsZb/UI-Client-Settings?node-id=0%3A1. Some check boxes there use different text depending on context. I would not mind removing the customisability of that if it helps speed things up.

arflyte commented 1 month ago

I'd imagined for the long screen, it'd be in two columns. As for the idea of the changing text is to provide more context of that it's doing for certain things that has two options other than 'yes' or 'no' like "Classic" or "Modern" for example.

If we're not happy with the direction, best we keep the current style for the moment.

As for the switch location, after discussion with Nanaya today, probably it's best for me to explore moving the button to the other side.

peppy commented 1 month ago

like "Classic" or "Modern" for example.

I don't foresee this ever being used. That's not how we design boolean options.

bdach commented 1 month ago

I pushed a half-hearted commit to attempt to address the feedback above but I'm very unsure about the end result.

https://github.com/user-attachments/assets/a0910145-2f14-445b-adb5-2ef9ae7894b1

Maybe could be better if the nub didn't change width, but all other nubs across the game do that. Dunno.

peppy commented 1 month ago

Functional but looks pretty bad. I think having it on the right is better but we need to limit width of all usages to ensure it doesn't become screen-wide. Arguably we should be doing this in most places with form content (see this egregious example):

osu! 2024-09-13 at 10 19 06

First run setup does it (correctly) by limiting the full dialog size:

osu! 2024-09-13 at 10 19 38

but also just limiting the content width and centring it inside the containing box, or making it 2 column wide (more risky for handling 4:3 squashing though).


Put another way: if you're okay to make sure we limit the width of the editor form display, then just revert to the previous design, remove the localisation of the enabled/disabled state text, and make the test scene also limited width so it doesn't look silly and we should be good to go.