Multiple notes can now be selected with keyboard, by pressing Shift+A… #526
Conversation
| /// Handles events related to the multiselection of consecutive notes. Returns true if a fitting event was found and handled. | ||
| bool handleKeyboardNoteSelectionEvent(QKeyEvent* keyEvent); | ||
| /// A boolean helper flag for the above method. | ||
| bool myIsCurrentlyDoingNoteSelectionWithKeyboard; |
There was a problem hiding this comment.
I think this is uninitialized currently - just doing = false here would be easiest
Also, for a less verbose name perhaps something like myIsKeyboardSelectionActive?
There was a problem hiding this comment.
Yes, the name is indeed too long. As for the initial value: An (incorrect) assumption on my part, I was under the impression that bools are initialized to false - a quick search proved that wrong, thanks. Probably also a conflation of different languages.
| return QMainWindow::eventFilter(object, event); | ||
| } | ||
|
|
||
| // Anly handle key presses that influence the score area, i.e. if the score area is |
There was a problem hiding this comment.
Just to avoid having a misleading comment, getScoreArea() would be null only when there aren't any open documents (I don't think focus would affect it, that just affects whether key event make it to the root window's event handler)
| if (! myIsCurrentlyDoingNoteSelectionWithKeyboard) { | ||
| myIsCurrentlyDoingNoteSelectionWithKeyboard = true; | ||
| location.setSelectionStart(currentPosIndex); | ||
| getScoreArea()->getClickEvent().signal(ScoreItem::Staff, location, ScoreItemAction::Selected); |
There was a problem hiding this comment.
I haven't tested, but I'm not entirely sure if this is necessary - it feels a bit awkward to be faking a click event.
I would have thought you could just update the location's position index, unless that isn't signalling to redraw the selection or something like that?
There was a problem hiding this comment.
Looking at this a bit more, I think you want to go through the Caret class since its onLocationChanged event will trigger the selection to redraw
Its existing methods might need some extensions to work here since many of them like moveHorizontal are intended for the normal arrow key events, which clear out the selection .. but perhaps adding some extra flags to the necessary methods would be sufficient.
Or, if you create a copy of the ScoreLocation and set it to have the appropriate position and selection start position, just use Caret::moveToLocation()
| } | ||
|
|
||
| int direction = (keyEvent->key() == Qt::Key_Left) ? -1 : 1; | ||
| int newPosIndex = fmin(fmax(currentPosIndex + direction, 0), |
There was a problem hiding this comment.
Something like getCaret().moveHorizontal() might work here to avoid doing all the clamping yourself (also, using std::min rather than the floating point fmin() would be preferable if it is necessary)
|
Thank for looking at it so quickly, @cameronwhite ! I will go through your comments later. This is in fact my first contribution to Open Source - I am a working developer, but have not found any obvious project I could/wanted to contribute to until just now :) Also, as my day job only involves Objective-C, Swift, and Java, I have to admit that I am not very experienced in the two main players in this project:
But I hope I can pick those, along with familiarization with the actual project, up quickly to do some helpful work. |
|
That's great! Always nice to hear from people that were on the old forums :) |
2 participants
Previously, for everything that operated on multiple notes (copying, irregular groupings, let ring, palm mute, etc ...), those notes had to be selected by mouse.
Now one does not have to leave the keyboard, but can instead start selecting with Shift+Alt (Shift+Opt on macOS), and then navigate the caret left or right using the arrow keys to extend the selection.
Note: I have only tested this on a macbook.