Fix song select navigation with page up/down#36293
Merged
peppy merged 3 commits intoppy:masterfrom Mar 1, 2026
Merged
Conversation
Signed-off-by: Linus Genz <linuslinuxgenz@gmail.com>
Member
|
Duplicate of #36305. Only one of these two should be merged. |
|
Now supersedes #36305 |
peppy
requested changes
Mar 1, 2026
| [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectNext))] | ||
| SelectNext, | ||
|
|
||
| [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectPreviousPage))] |
Member
There was a problem hiding this comment.
I can't think of a case someone would want to customise this. Let's just code it local to song select as pageup/pagedown?
Member
There was a problem hiding this comment.
Also you missed the comment on the enum where what you've done here will break the game very obviously. Running the game will immediately show the breakage of hotkeys.
| new KeyBinding(InputKey.Up, GlobalAction.SelectPrevious), | ||
| new KeyBinding(InputKey.Down, GlobalAction.SelectNext), | ||
|
|
||
| new KeyBinding(InputKey.PageUp, GlobalAction.SelectPreviousPage), |
Member
There was a problem hiding this comment.
These are in global but only ever used for song select which is misleading. In addition, if you change the bindings, page up and page down will now perform different behaviour but still work.
peppy
force-pushed
the
36099-song-select-with-page-keys
branch
from
12fa234 to
de8d47f
Compare
Member
|
Applied required changes myself. |
peppy
approved these changes
Mar 1, 2026
Rudicito
added a commit
to Rudicito/osu
that referenced
this pull request
Mar 7, 2026
It's a continuation of ppy#36293, but for home and end keys.
peppy
pushed a commit
that referenced
this pull request
Mar 10, 2026
It's a continuation of #36293, but for the home and end keys. Now when using home or end keys, it selects respectively the first or last item in the carousel, instead of just scrolling. ## Before: https://github.com/user-attachments/assets/6ab08d2f-1da4-4740-9d9e-574d7a8a10c9 ## After: https://github.com/user-attachments/assets/30bab836-0006-4830-b4e9-2d85017a15e6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #36099
This PR fixes keyboard navigation in the beatmap select carousel for lazer by implementing page-wise traversal with the Page Up and Page Down keys and changing it from only scrolling to actually selecting items.
Changes:
TraversalType.Pagein the keyboard traversal switch.traverseKeyboardPage(int direction)method to move the selection by approximately one "page" of visible items, accounting for partially obscured items like the search bar. Also it does not wrap around (like the current PageUp/Down functionality).PageUp→ SelectPreviousPagePageDown→ SelectNextPageThe code may be very explicit for the scroll logic with the page keys, so I would appreciate some feedback when the PR is reviewed.
The naming of the keybinds may need to be adjusted.
Next pageandprevious pagemay be somewhat misleading.Behavior after the change:
See:
https://www.youtube.com/watch?v=JXmKAhhKiCc