search

ppy / osu-framework

A game framework written with osu! in mind.
MIT License
1.67k stars 420 forks source link

Allow slider bar to take focus and accept keyboard input while not hovered #6390

Closed OliBomby closed 1 month ago

OliBomby commented 1 month ago

This change would allow you to click the slider bar, have it gain focus, and then adjust the slider bar with the keyboard while the cursor is not hovering the slider bar.

This matches the user expectation because you can do the same thing in osu! stable. In current lazer the only way to do this is click away to unfocus everything, then hover the slider bar while using keyboard. This flow is apparently so confusing that someone in Discord thought it wasn't possible at all.

Discord_XhhdbulGlk

https://github.com/user-attachments/assets/19d901c3-4130-48f8-be75-bedc75a6676f

Discord_5hnRVUNu9n

bdach commented 1 month ago

Also test failures

OliBomby commented 1 month ago

That said, it should have focus feedback. Not many UIs do this, but I feel like it's needed for lazer. In Windows 11, the volume control shows an outline with the number always visible when using keyboard:

Correct me if I'm wrong, but I think the visual feedback is to be fixed on lazer's side, not framework. I can follow up with a PR on lazer for focus feedback after this is merged.

Joehuu commented 1 month ago

Yes, but the visual can be basic, to demonstrate the behavior and implementation here (in BasicSliderBar). It's mostly for framework consumers, and it'll not be as subjective as lazer's side.

OliBomby commented 1 month ago

You're so right. How didnt i notice this

peppy commented 1 month ago

The Basic display's active state is quite shoddy. Haven't dived into the code yet.

peppy commented 1 month ago

There's no osu! side PR to see how this will look yet is there?

bdach commented 1 month ago

https://github.com/ppy/osu/pull/30315 was vaguely linked but does not actually use anything from this PR or include any visual changes to sliders to indicate focus.

peppy commented 1 month ago

I think I'd want to see that, just to confirm everything is as it should be, before going ahead with this.

Also arguably, this is changing the UX of this control and other framework consumers may prefer the old behaviour. I'm relatively neutral on it, but maybe we want focus to be given when hovering and arrow keys are used to keep the old behaviour also working?

OliBomby commented 1 month ago

maybe we want focus to be given when hovering and arrow keys are used to keep the old behaviour also working?

This suggestion confuses me. The way I have it implemented now does allow you to use the old behaviour, that is adjusting the slider bar with keyboard while hovering without focusing. To allow keyboard input it needs either focus or hover state. To give (persisting) focus when hovering is not necessary and its not how it works in OSes, which I aim to mimic.

peppy commented 1 month ago

Right, it has been fixed in the latest commits.

peppy commented 1 month ago

I'm tentatively okay with this, but would still want to see the osu!-side change before merging.