Percussion keyboard shortcuts #26222
Merged
Conversation
This file contains 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
mathesoncalum
force-pushed
the
percussion_keyboard_shortcuts
branch
4 times, most recently
from
dcb3ee8 to
6e4ba68
Compare
mathesoncalum
force-pushed
the
percussion_keyboard_shortcuts
branch
3 times, most recently
from
38853cc to
e88e5ef
Compare
mathesoncalum
requested review from
Eism
and removed request for
cbjeukendrup
mathesoncalum
force-pushed
the
percussion_keyboard_shortcuts
branch
from
e88e5ef to
4fa176c
Compare
Eism
reviewed
Jan 30, 2025
Eism
reviewed
Jan 30, 2025
src/notation/qml/MuseScore/NotationScene/EditPercussionShortcutDialog.qml
Outdated
Show resolved
Hide resolved
…::shortcutOverrideEvent
mathesoncalum
force-pushed
the
percussion_keyboard_shortcuts
branch
from
4fa176c to
3b4df9b
Compare
RomanPudashkin
approved these changes
Feb 6, 2025
|
Percussion shortcuts break combo key commands that work in Master when percussion stave is selected. Shift+shortcut override is expected
|
mathesoncalum
force-pushed
the
percussion_keyboard_shortcuts
branch
from
1d97411 to
dce242c
Compare
|
Tested on MacOS 15, Windows 11, Ubuntu 22.04.3. Approved |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR implements our new-and-improved percussion shortcuts. After these changes, it will be possble to use keys beyond
ABCDEFGto input percussion notes.PR Overview
As outlined in the spec for this task, choosing “Define keyboard shortcut” from the percussion panel or editing shortcuts from the Customize kit dialog should prompt the user for an updated shortcut.
While the current flow provided by
EditShortcutModelis quite similar to that of the new spec, it's clear that a more specialised model is required for percussion shortcuts (adding extra percussion-specific logic toEditShortcutModelprobably wouldn't be appropriate/coherent, especially considering it's part of the framework module). The logic we're interested in also requires some decoupling from preferences.To address this, 7984f8b extracts the content of our current
EditShortcutDialoginto anEditShortcutDialogContentcomponent which can be re-used with other models across the application. The second commit here (7219ade) implements theEditPercussionShortcutDialogwith an associatedEditPercussionShortcutModeland provides a logic for opening it inPercussionUtilities.Commit 6e4ba68 replaces the current shortcut dropdown in the customize kit dialog with a button that triggers the new flow, while commits cff5256 and 4fa176c lay the groundwork for actually using the expanded range of shortcuts for note input.
Finally, dce242c uses
NoteInputParamsandhandleNoteEventto dispatch percussion shortcuts.