Skip to content
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.

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

Replace explicit library home/end key handlers with a moveCursor override #11100

Merged
merged 4 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 65 additions & 32 deletions src/widget/wlibrarytableview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,50 +263,83 @@ void WLibraryTableView::focusInEvent(QFocusEvent* event) {

QModelIndex WLibraryTableView::moveCursor(CursorAction cursorAction,
Qt::KeyboardModifiers modifiers) {
// The up and down cursor keys should wrap the list around. This behavior
// also applies to the `[Library],MoveVertical` action that is usually bound
// to the library browse encoder on controllers. Otherwise browsing a
// key-sorted library list requires either a serious workout or the user
// needs to reach for the mouse or keyboard when moving between 12/C#m/E and
// 1/G#m/B.
QAbstractItemModel* pModel = model();
if (pModel &&
(cursorAction == QAbstractItemView::MoveUp ||
cursorAction == QAbstractItemView::MoveDown)) {
// This is very similar to `moveSelection()`, except that it doesn't
// actually modify the selection
const QModelIndex current = currentIndex();
if (current.isValid()) {
const int row = currentIndex().row();
const int column = currentIndex().column();
if (cursorAction == QAbstractItemView::MoveDown) {
if (row + 1 < pModel->rowCount()) {
return pModel->index(row + 1, column);
if (pModel) {
Copy link
Member

Choose a reason for hiding this comment

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

Early return if !pModel? Probably a matter of taste, but it reduces the indentation below.

Suggested change
if (pModel) {
if (!pModel) {
return QTableView::moveCursor(cursorAction, modifiers);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I didn't want to repeat the base case. Felt this was similar to something like a Win32 window procedure where you fall back to the default behavior if you didn't return early.

switch (cursorAction) {
// The up and down cursor keys should wrap the list around. This
// behavior also applies to the `[Library],MoveVertical` action that is
// usually bound to the library browse encoder on controllers. Otherwise
// browsing a key-sorted library list requires either a serious workout
// or the user needs to reach for the mouse or keyboard when moving
// between 12/C#m/E and 1/G#m/B. This is very similar to
// `moveSelection()`, except that it doesn't actually modify the
// selection. It simply returns a new cursor that the keyboard event
// handler in `QAbstractItemView` uses to either move the cursor, move
// the selection, or extend the selection depending on which modifier
// keys are held down.
case QAbstractItemView::MoveUp:
case QAbstractItemView::MoveDown: {
const QModelIndex current = currentIndex();
if (current.isValid()) {
const int row = currentIndex().row();
const int column = currentIndex().column();
if (cursorAction == QAbstractItemView::MoveDown) {
if (row + 1 < pModel->rowCount()) {
return pModel->index(row + 1, column);
} else {
return pModel->index(0, column);
}
} else {
return pModel->index(0, column);
if (row - 1 >= 0) {
return pModel->index(row - 1, column);
} else {
return pModel->index(pModel->rowCount() - 1, column);
}
}
} else {
if (row - 1 >= 0) {
return pModel->index(row - 1, column);
} else {
return pModel->index(pModel->rowCount() - 1, column);
// If the cursor does not yet exist (because the view has not
// yet been interacted with) then this selects the first or last
// row
const int row = cursorAction == QAbstractItemView::MoveUp
? pModel->rowCount() - 1
: 0;

// Selecting a hidden column doesn't work, so we'll need to find
// the first non-hidden column here
int column = 0;
while (isColumnHidden(column) && column < pModel->columnCount()) {
column++;
}

return pModel->index(row, column);
}
} else {
// Selecting a hidden column doesn't work, so we'll need to find the
// first non-hidden column here
int column = 0;
while (isColumnHidden(column) && column < pModel->columnCount()) {
column++;
} break;
// Make the home and end keys move to the first and last row rather than
// the first and last column (QAbstractItemView default)
case QAbstractItemView::MoveHome:
case QAbstractItemView::MoveEnd: {
const QModelIndex current = currentIndex();

// We don't want to change the selected column if a column has
// already been selected
int column = current.column();
if (!current.isValid()) {
// Selecting a hidden column doesn't work, so we'll need to find
// the first non-hidden column here
int column = 0;
while (isColumnHidden(column) && column < pModel->columnCount()) {
column++;
}
}

// If the cursor does not yet exist (because the view has not yet
// been interacted with) then this selects the first/last row
if (cursorAction == QAbstractItemView::MoveDown) {
if (cursorAction == QAbstractItemView::MoveHome) {
return pModel->index(0, column);
} else {
return pModel->index(pModel->rowCount() - 1, column);
}
} break;
default:
break;
}
}

Expand Down
29 changes: 0 additions & 29 deletions src/widget/wtracktableview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -819,35 +819,6 @@ void WTrackTableView::keyPressEvent(QKeyEvent* event) {
return;
}
} break;
case Qt::Key_Home: { // Jump to first row
if (model()->rowCount() == 0) {
return;
}
int currCol = 0;
QModelIndex currIdx = currentIndex();
if (currIdx.isValid()) {
currCol = currIdx.column();
}
QModelIndex newIdx = model()->index(0, currCol);
selectionModel()->setCurrentIndex(newIdx,
QItemSelectionModel::SelectCurrent | QItemSelectionModel::Rows);
scrollTo(newIdx);
} break;
case Qt::Key_End: { // Jump to last row
const int lastRow = model()->rowCount() - 1;
if (lastRow == -1) {
return;
}
int currCol = 0;
QModelIndex currIdx = currentIndex();
if (currIdx.isValid()) {
currCol = currIdx.column();
}
QModelIndex newIdx = model()->index(lastRow, currCol);
selectionModel()->setCurrentIndex(newIdx,
QItemSelectionModel::SelectCurrent | QItemSelectionModel::Rows);
scrollTo(newIdx);
} break;
default:
QTableView::keyPressEvent(event);
}
Expand Down