Skip to content

Multiple notes can now be selected with keyboard, by pressing Shift+A… #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zerschmetterling wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Multiple notes can now be selected with keyboard, by pressing Shift+A… #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 59 additions & 3 deletions source/app/powertabeditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2072,11 +2072,37 @@ bool PowerTabEditor::eventFilter(QObject *object, QEvent *event)
if (myIsPlaying)
return QMainWindow::eventFilter(object, event);

// Only handle key events.
QKeyEvent *keyEvent = dynamic_cast<QKeyEvent *>(event);
if (keyEvent == nullptr) {
return QMainWindow::eventFilter(object, event);
}

// Anly handle key presses that influence the score area, i.e. if the score area is
Copy link
Member

@cameronwhite cameronwhite Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

// the active widget
ScoreArea *scorearea = getScoreArea();
if (scorearea && event->type() == QEvent::KeyPress)
if (!scorearea)
{
QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);
if (keyEvent->key() >= Qt::Key_0 && keyEvent->key() <= Qt::Key_9)
return QMainWindow::eventFilter(object, event);
}


if (event->type() == QEvent::KeyRelease)
{
if (keyEvent->key() == Qt::Key_Shift || keyEvent->key() == Qt::Key_Alt)
{
// If either of these keys are released, the current selection cycle should end.
myIsCurrentlyDoingNoteSelectionWithKeyboard = false;
}
}
else if (event->type() == QEvent::KeyPress)
{
// Method call returns true if selection event was handled.
if (handleKeyboardNoteSelectionEvent(keyEvent))
{
return true;
}
else if (keyEvent->key() >= Qt::Key_0 && keyEvent->key() <= Qt::Key_9)
{
const int number = keyEvent->key() - Qt::Key_0;
ScoreLocation &location = getLocation();
Expand Down Expand Up @@ -2136,6 +2162,36 @@ bool PowerTabEditor::eventFilter(QObject *object, QEvent *event)
return QMainWindow::eventFilter(object, event);
}

bool PowerTabEditor::handleKeyboardNoteSelectionEvent(QKeyEvent* keyEvent)
{
// Keyboard selection should only be handled if all of 'Shift + Alt + an arrow key' are pressed,
// and only if the arrow key is left or right.
Qt::KeyboardModifiers selectionModifiers = Qt::ShiftModifier | Qt::AltModifier | Qt::KeypadModifier;

if (keyEvent->keyCombination().keyboardModifiers() != selectionModifiers
|| (keyEvent->key() != Qt::Key_Left && keyEvent->key() != Qt::Key_Right)) {
return false;
}

ScoreLocation& location = getLocation();
int currentPosIndex = location.getPositionIndex();

// If this is the first selection event, we set the selection start, and signal selection of
// the current position before advancing the caret.
if (! myIsCurrentlyDoingNoteSelectionWithKeyboard) {
myIsCurrentlyDoingNoteSelectionWithKeyboard = true;
location.setSelectionStart(currentPosIndex);
getScoreArea()->getClickEvent().signal(ScoreItem::Staff, location, ScoreItemAction::Selected);
Copy link
Member

@cameronwhite cameronwhite Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@cameronwhite cameronwhite Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),
Copy link
Member

@cameronwhite cameronwhite Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

location.getSystem().getBarlines().back().getPosition() - 1);
location.setPositionIndex(newPosIndex);
getScoreArea()->getClickEvent().signal(ScoreItem::Staff, location, ScoreItemAction::Selected);
return true;
}

void PowerTabEditor::closeEvent(QCloseEvent *event)
{
while (myTabWidget->currentIndex() != -1)
Expand Down
9 changes: 8 additions & 1 deletion source/app/powertabeditor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ private slots:
void info(void);

protected:
/// Handle key presses for 0-9 when entering tab numbers.
/// Main event handler. Currently handles:
/// - key presses for 0-9 when entering tab numbers.
/// - key presses related to multi-selection of notes (see dedicated method ``handleKeyboardNoteSelectionEvent``).
virtual bool eventFilter(QObject *object, QEvent *event) override;

/// Performs some final actions before exiting.
Expand Down Expand Up @@ -420,6 +422,11 @@ private slots:
/// Increases or decreases the line spacing by the given amount.
void adjustLineSpacing(int amount);

/// 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;
Copy link
Member

@cameronwhite cameronwhite Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is uninitialized currently - just doing = false here would be easiest

Also, for a less verbose name perhaps something like myIsKeyboardSelectionActive?

Copy link
Author

@zerschmetterling zerschmetterling Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/// Returns the score area for the active document.
ScoreArea *getScoreArea();
/// Returns the caret for the active document.
Expand Down
Loading