Skip to content

Commit

Permalink
QTableView: Fix keyboard navigation with disabled rows
Browse files Browse the repository at this point in the history 
The keyboard navigation with MovePageUp/Down and MoveEnd did not honor
disabled cells in all cases which lead to inconsistencies in the
navigation (esp. since MoveHome does honor them correctly).
Therefore make sure that all four move operations work consistent by
refactoring the code to use common functions.

Fixes: QTBUG-72400
Change-Id: I63fa3b626510d21c66f4f9b2b1bfb3261728ecaf
Reviewed-by: Friedemann Kleint <[email protected]>
Reviewed-by: Luca Beldi <[email protected]>
Reviewed-by: Samuel Gaist <[email protected]>
Reviewed-by: Richard Moe Gustavsen <[email protected]>
  • Loading branch information
@chehrlic
chehrlic committed Dec 18, 2018
1 parent 53ae00d commit 7863be3
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 38 deletions.
103 changes: 81 additions & 22 deletions src/widgets/itemviews/qtableview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,66 @@ bool QTableViewPrivate::spanContainsSection(const QHeaderView *header, int logic
return false;
}

/*!
\internal
Searches for the next cell which is available for e.g. keyboard navigation
The search is done by row
*/
int QTableViewPrivate::nextActiveVisualRow(int rowToStart, int column, int limit,
SearchDirection searchDirection) const
{
const int lc = logicalColumn(column);
int visualRow = rowToStart;
const auto isCellActive = [this](int vr, int lc)
{
const int lr = logicalRow(vr);
return !isRowHidden(lr) && isCellEnabled(lr, lc);
};
switch (searchDirection) {
case SearchDirection::Increasing:
if (visualRow < limit) {
while (!isCellActive(visualRow, lc)) {
if (++visualRow == limit)
return rowToStart;
}
}
break;
case SearchDirection::Decreasing:
while (visualRow > limit && !isCellActive(visualRow, lc))
--visualRow;
break;
}
return visualRow;
}

/*!
\internal
Searches for the next cell which is available for e.g. keyboard navigation
The search is done by column
*/
int QTableViewPrivate::nextActiveVisualColumn(int row, int columnToStart, int limit,
SearchDirection searchDirection) const
{
const int lr = logicalRow(row);
int visualColumn = columnToStart;
const auto isCellActive = [this](int lr, int vc)
{
const int lc = logicalColumn(vc);
return !isColumnHidden(lc) && isCellEnabled(lr, lc);
};
switch (searchDirection) {
case SearchDirection::Increasing:
while (visualColumn < limit && !isCellActive(lr, visualColumn))
++visualColumn;
break;
case SearchDirection::Decreasing:
while (visualColumn > limit && !isCellActive(lr, visualColumn))
--visualColumn;
break;
}
return visualColumn;
}

/*!
\internal
Returns the visual rect for the given \a span.
Expand Down Expand Up @@ -1800,35 +1860,34 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi
break;
}
case MoveHome:
visualColumn = 0;
while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn))
++visualColumn;
if (modifiers & Qt::ControlModifier) {
visualRow = 0;
while (visualRow < bottom && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn))
++visualRow;
}
visualColumn = d->nextActiveVisualColumn(visualRow, 0, right,
QTableViewPrivate::SearchDirection::Increasing);
if (modifiers & Qt::ControlModifier)
visualRow = d->nextActiveVisualRow(0, visualColumn, bottom,
QTableViewPrivate::SearchDirection::Increasing);
break;
case MoveEnd:
visualColumn = right;
visualColumn = d->nextActiveVisualColumn(visualRow, right, -1,
QTableViewPrivate::SearchDirection::Decreasing);
if (modifiers & Qt::ControlModifier)
visualRow = bottom;
visualRow = d->nextActiveVisualRow(bottom, current.column(), -1,
QTableViewPrivate::SearchDirection::Decreasing);
break;
case MovePageUp: {
int newRow = rowAt(visualRect(current).bottom() - d->viewport->height());
if (newRow == -1) {
int visualRow = 0;
while (visualRow < bottom && isRowHidden(d->logicalRow(visualRow)))
++visualRow;
newRow = d->logicalRow(visualRow);
}
return d->model->index(newRow, current.column(), d->root);
int newLogicalRow = rowAt(visualRect(current).bottom() - d->viewport->height());
int visualRow = (newLogicalRow == -1 ? 0 : d->visualRow(newLogicalRow));
visualRow = d->nextActiveVisualRow(visualRow, current.column(), bottom,
QTableViewPrivate::SearchDirection::Increasing);
newLogicalRow = d->logicalRow(visualRow);
return d->model->index(newLogicalRow, current.column(), d->root);
}
case MovePageDown: {
int newRow = rowAt(visualRect(current).top() + d->viewport->height());
if (newRow == -1)
newRow = d->logicalRow(bottom);
return d->model->index(newRow, current.column(), d->root);
int newLogicalRow = rowAt(visualRect(current).top() + d->viewport->height());
int visualRow = (newLogicalRow == -1 ? bottom : d->visualRow(newLogicalRow));
visualRow = d->nextActiveVisualRow(visualRow, current.column(), -1,
QTableViewPrivate::SearchDirection::Decreasing);
newLogicalRow = d->logicalRow(visualRow);
return d->model->index(newLogicalRow, current.column(), d->root);
}}

d->visualCursor = QPoint(visualColumn, visualRow);
Expand Down
20 changes: 10 additions & 10 deletions src/widgets/itemviews/qtableview_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,16 +234,16 @@ class QTableViewPrivate : public QAbstractItemViewPrivate
inline bool isCellEnabled(int row, int column) const {
return isIndexEnabled(model->index(row, column, root));
}
inline bool isVisualRowHiddenOrDisabled(int row, int column) const {
int r = logicalRow(row);
int c = logicalColumn(column);
return isRowHidden(r) || !isCellEnabled(r, c);
}
inline bool isVisualColumnHiddenOrDisabled(int row, int column) const {
int r = logicalRow(row);
int c = logicalColumn(column);
return isColumnHidden(c) || !isCellEnabled(r, c);
}

enum class SearchDirection
{
Increasing,
Decreasing
};
int nextActiveVisualRow(int rowToStart, int column, int limit,
SearchDirection searchDirection) const;
int nextActiveVisualColumn(int row, int columnToStart, int limit,
SearchDirection searchDirection) const;

QRect visualSpanRect(const QSpanCollection::Span &span) const;

Expand Down
40 changes: 34 additions & 6 deletions tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1264,19 +1264,47 @@ void tst_QTableView::moveCursorStrikesBack_data()
for (int i = 0; i < 7; ++i)
fullList << i;

QTest::newRow("All disabled, wrap forward. Timeout => FAIL") << -1 << -1
QTest::newRow("All disabled, wrap forward. => invalid index") << -1 << -1
<< fullList
<< fullList
<< QRect()
<< 1 << 0 << (IntList() << int(QtTestTableView::MoveNext))
<< 1 << 0;
<< -1 << -1;

QTest::newRow("All disabled, wrap backwards. Timeout => FAIL") << -1 << -1
QTest::newRow("All disabled, wrap backwards. => invalid index") << -1 << -1
<< fullList
<< fullList
<< QRect()
<< 1 << 0 << (IntList() << int(QtTestTableView::MovePrevious))
<< -1 << -1;

QTest::newRow("Last column disabled, MoveEnd. QTBUG-72400") << -1 << -1
<< IntList()
<< (IntList() << 6)
<< QRect()
<< 0 << 0 << (IntList() << int(QtTestTableView::MoveEnd))
<< 0 << 5;

QTest::newRow("First column disabled, MoveHome. QTBUG-72400") << -1 << -1
<< IntList()
<< (IntList() << 0)
<< QRect()
<< 0 << 6 << (IntList() << int(QtTestTableView::MoveHome))
<< 0 << 1;

QTest::newRow("First row disabled, MovePageUp. QTBUG-72400") << -1 << -1
<< (IntList() << 0)
<< IntList()
<< QRect()
<< 2 << 0 << (IntList() << int(QtTestTableView::MovePageUp))
<< 1 << 0;

QTest::newRow("Last row disabled, MovePageDown. QTBUG-72400") << -1 << -1
<< (IntList() << 6)
<< IntList()
<< QRect()
<< 4 << 0 << (IntList() << int(QtTestTableView::MovePageDown))
<< 5 << 0;
}

void tst_QTableView::moveCursorStrikesBack()
Expand All @@ -1302,6 +1330,9 @@ void tst_QTableView::moveCursorStrikesBack()
if (span.height() && span.width())
view.setSpan(span.top(), span.left(), span.height(), span.width());
view.show();
QVERIFY(QTest::qWaitForWindowActive(&view));
// resize to make sure there are scrollbars
view.resize(view.columnWidth(0) * 7, view.rowHeight(0) * 7);

QModelIndex index = model.index(startRow, startColumn);
view.setCurrentIndex(index);
Expand All @@ -1320,9 +1351,6 @@ void tst_QTableView::moveCursorStrikesBack()
newColumn = newIndex.column();
}

// expected fails, task 119433
if(newRow == -1)
return;
QCOMPARE(newRow, expectedRow);
QCOMPARE(newColumn, expectedColumn);
}
Expand Down

0 comments on commit 7863be3

Please sign in to comment.