CaffeineMC / sodium-fabric

A Minecraft mod designed to improve frame rates and reduce micro-stutter
Other
4.74k stars 809 forks source link

Space cannot be used to activate UI elements on Sodium screens #2275

Open haykam821 opened 8 months ago

haykam821 commented 8 months ago

In a recent Minecraft version, the user interface was updated to allow for space to be pressed to activate elements in addition to the previously-working enter key. However, Sodium doesn't appear to respond to space for keyboard navigation, only enter.

The root cause seems to be various checks for the enter key specifically:

https://github.com/CaffeineMC/sodium-fabric/blob/c937156e37d221216f284639fc8c51b30facf91e/src/main/java/me/jellysquid/mods/sodium/client/gui/options/control/TickBoxControl.java#L83 https://github.com/CaffeineMC/sodium-fabric/blob/c937156e37d221216f284639fc8c51b30facf91e/src/main/java/me/jellysquid/mods/sodium/client/gui/widgets/FlatButtonWidget.java#L88 https://github.com/CaffeineMC/sodium-fabric/blob/c937156e37d221216f284639fc8c51b30facf91e/src/main/java/me/jellysquid/mods/sodium/client/gui/options/control/CyclingControl.java#L115

Vanilla uses the KeyCodes.isToggle method to check whether a key is space, enter, or keypad enter within widgets; the method should be a direct replacement for the above checks. I am willing to open a pull request to fix this issue.

This issue can be reproduced with Minecraft 1.20.4 and the build of the latest commit as of this issue: 0.5.6+git.c937156 from c937156e37d221216f284639fc8c51b30facf91e.

jellysquid3 commented 8 months ago

Ah, seems like an oversight during porting. I think we might have also had some issue where using SPACE caused other interaction issues. Needs to be investigated again.

haykam821 commented 8 months ago

From what I can tell, keyboard navigation and particularly the space key works fine, though there are some slight oddities compared to vanilla I've noticed otherwise that could also be improved:

Those issues don't seem to affect the space key specifically, so they can be fixed independently, and I am also willing to open pull requests for those issues. I can also open groups of issues so that these issues can be better tracked (tabs could be solved in one go, for example). The high contrast issue in particular would seem to need extra care since the UI elements are manually drawn rather than from a texture.

jellysquid3 commented 8 months ago

This issue was incorrectly closed by GitHub on merge because it was linked to the pull request.

haykam821 commented 8 months ago

From above, I can group the tab issues into a larger pull request since the tab issues are somewhat intertwined. For example, the focus blurring is likely related to tab not working the first time after changing a tab, and both of those issues are likely caused by the screen's children being cleared when changing tabs.

High contrast being unsupported is now covered by #2306.

That leaves the following issues from the above list which are minor and don't impede any keyboard navigation but are still worth fixing:

jellysquid3 commented 8 months ago

Thanks for looking into it. If you want to contribute to fix the issues with keyboard navigation, feel free to. Larger pull requests are not an issue per-say but we generally prefer to have individual commits in the tree for each problem fixed.

Unfortunately, the GUI code is a bit of a mess and I'm not sure how difficult it will be. But if you do open pull requests, we'll try to merge into the Sodium 0.5.x branches rather than defer to Sodium 0.6.

I've been trying to open additional issues from this one (since having evergreen issues is generally not good) and have created the A-accessibility label to cover them. When everything else has been opened, we'll close this one.