From 8da7252de0badb818302763cbe62c38ad699f1f3 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Mon, 12 Oct 2009 14:28:33 +0200 Subject: Fixed keyboard navigation for QTableView Keyboard navigation didn't work in the following cases: - The last column was disabled and we pressed the tab key when at the 2nd last column. (See ref. task). - Spans with their anchor column or row hidden or disabled. - Navigation would not preserve the original row/column when traversing a span horizontally/vertically. Auto-tests submitted with this commit. Task-number: QTBUG-3878 Reviewed-by: Olivier --- src/gui/itemviews/qabstractitemview.cpp | 3 + src/gui/itemviews/qtableview.cpp | 204 ++++++++++++++++++++----------- src/gui/itemviews/qtableview_p.h | 4 +- tests/auto/qtableview/tst_qtableview.cpp | 166 ++++++++++++++++++++++++- 4 files changed, 304 insertions(+), 73 deletions(-) diff --git a/src/gui/itemviews/qabstractitemview.cpp b/src/gui/itemviews/qabstractitemview.cpp index 37f4184..268e78e 100644 --- a/src/gui/itemviews/qabstractitemview.cpp +++ b/src/gui/itemviews/qabstractitemview.cpp @@ -2185,6 +2185,9 @@ void QAbstractItemView::keyPressEvent(QKeyEvent *event) } else { d->selectionModel->setCurrentIndex(newCurrent, command); d->pressedPosition = visualRect(newCurrent).center() + d->offset(); + // We copy the same behaviour as for mousePressEvent(). + QRect rect(d->pressedPosition - d->offset(), QSize(1, 1)); + setSelection(rect, command); } return; } diff --git a/src/gui/itemviews/qtableview.cpp b/src/gui/itemviews/qtableview.cpp index 15bd445..2d98258 100644 --- a/src/gui/itemviews/qtableview.cpp +++ b/src/gui/itemviews/qtableview.cpp @@ -779,8 +779,6 @@ void QTableViewPrivate::drawAndClipSpans(const QRegion &area, QPainter *painter, foreach (QSpanCollection::Span *span, visibleSpans) { int row = span->top(); int col = span->left(); - if (isHidden(row, col)) - continue; QModelIndex index = model->index(row, col, root); if (!index.isValid()) continue; @@ -1480,12 +1478,30 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi ++column; while (isRowHidden(d->logicalRow(row)) && row < bottom) ++row; + d->visualCursor = QPoint(column, row); return d->model->index(d->logicalRow(row), d->logicalColumn(column), d->root); } - int visualRow = d->visualRow(current.row()); + // Update visual cursor if current index has changed. + QPoint visualCurrent(d->visualColumn(current.column()), d->visualRow(current.row())); + if (visualCurrent != d->visualCursor) { + if (d->hasSpans()) { + QSpanCollection::Span span = d->span(current.row(), current.column()); + if (span.top() > d->visualCursor.y() || d->visualCursor.y() > span.bottom() + || span.left() > d->visualCursor.x() || d->visualCursor.x() > span.right()) + d->visualCursor = visualCurrent; + } else { + d->visualCursor = visualCurrent; + } + } + + int visualRow = d->visualCursor.y(); + if (visualRow > bottom) + visualRow = bottom; Q_ASSERT(visualRow != -1); - int visualColumn = d->visualColumn(current.column()); + int visualColumn = d->visualCursor.x(); + if (visualColumn > right) + visualColumn = right; Q_ASSERT(visualColumn != -1); if (isRightToLeft()) { @@ -1496,22 +1512,33 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi } switch (cursorAction) { - case MoveUp: + case MoveUp: { + int originalRow = visualRow; #ifdef QT_KEYPAD_NAVIGATION if (QApplication::keypadNavigationEnabled() && visualRow == 0) visualRow = d->visualRow(model()->rowCount() - 1) + 1; + // FIXME? visualRow = bottom + 1; #endif - --visualRow; - while (visualRow > 0 && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn)) + int r = d->logicalRow(visualRow); + int c = d->logicalColumn(visualColumn); + if (r != -1 && d->hasSpans()) { + QSpanCollection::Span span = d->span(r, c); + if (span.width() > 1 || span.height() > 1) + visualRow = d->visualRow(span.top()); + } + while (visualRow >= 0) { --visualRow; - if (d->hasSpans()) { - int row = d->logicalRow(visualRow); - QSpanCollection::Span span = d->span(row, current.column()); - visualRow = d->visualRow(span.top()); - visualColumn = d->visualColumn(span.left()); + r = d->logicalRow(visualRow); + c = d->logicalColumn(visualColumn); + if (r == -1 || (!isRowHidden(r) && d->isCellEnabled(r, c))) + break; } + if (visualRow < 0) + visualRow = originalRow; break; - case MoveDown: + } + case MoveDown: { + int originalRow = visualRow; if (d->hasSpans()) { QSpanCollection::Span span = d->span(current.row(), current.column()); visualRow = d->visualRow(d->rowSpanEndLogical(span.top(), span.height())); @@ -1520,71 +1547,106 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi if (QApplication::keypadNavigationEnabled() && visualRow >= bottom) visualRow = -1; #endif - ++visualRow; - while (visualRow < bottom && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn)) + int r = d->logicalRow(visualRow); + int c = d->logicalColumn(visualColumn); + if (r != -1 && d->hasSpans()) { + QSpanCollection::Span span = d->span(r, c); + if (span.width() > 1 || span.height() > 1) + visualRow = d->visualRow(d->rowSpanEndLogical(span.top(), span.height())); + } + while (visualRow <= bottom) { ++visualRow; - if (d->hasSpans()) { - int row = d->logicalRow(visualRow); - QSpanCollection::Span span = d->span(row, current.column()); - visualColumn = d->visualColumn(span.left()); + r = d->logicalRow(visualRow); + c = d->logicalColumn(visualColumn); + if (r == -1 || (!isRowHidden(r) && d->isCellEnabled(r, c))) + break; } + if (visualRow > bottom) + visualRow = originalRow; break; - case MovePrevious: { - int left = 0; - while (d->isVisualColumnHiddenOrDisabled(visualRow, left) && left < right) - ++left; - if (visualColumn == left) { - visualColumn = right; - int top = 0; - while (top < bottom && d->isVisualRowHiddenOrDisabled(top, visualColumn)) - ++top; - if (visualRow == top) + } + case MovePrevious: + case MoveLeft: { + int originalRow = visualRow; + int originalColumn = visualColumn; + bool firstTime = true; + bool looped = false; + bool wrapped = false; + do { + int r = d->logicalRow(visualRow); + int c = d->logicalColumn(visualColumn); + if (firstTime && c != -1 && d->hasSpans()) { + firstTime = false; + QSpanCollection::Span span = d->span(r, c); + if (span.width() > 1 || span.height() > 1) + visualColumn = d->visualColumn(span.left()); + } + while (visualColumn >= 0) { + --visualColumn; + r = d->logicalRow(visualRow); + c = d->logicalColumn(visualColumn); + if (r == -1 || c == -1 || (!isRowHidden(r) && !isColumnHidden(c) && d->isCellEnabled(r, c))) + break; + if (wrapped && (originalRow < visualRow || (originalRow == visualRow && originalColumn <= visualColumn))) { + looped = true; + break; + } + } + if (cursorAction == MoveLeft || visualColumn >= 0) + break; + visualColumn = right + 1; + if (visualRow == 0) { + wrapped == true; visualRow = bottom; - else - --visualRow; - while (visualRow > 0 && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn)) + } else { --visualRow; - break; - } // else MoveLeft - } - case MoveLeft: - --visualColumn; - while (visualColumn > 0 && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn)) - --visualColumn; - if (d->hasSpans()) { - int column = d->logicalColumn(visualColumn); - QSpanCollection::Span span = d->span(current.row(), column); - visualRow = d->visualRow(span.top()); - visualColumn = d->visualColumn(span.left()); - } + } + } while (!looped); + if (visualColumn < 0) + visualColumn = originalColumn; break; + } case MoveNext: - if (visualColumn == right) { - visualColumn = 0; - while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn)) + case MoveRight: { + int originalRow = visualRow; + int originalColumn = visualColumn; + bool firstTime = true; + bool looped = false; + bool wrapped = false; + do { + int r = d->logicalRow(visualRow); + int c = d->logicalColumn(visualColumn); + if (firstTime && c != -1 && d->hasSpans()) { + firstTime = false; + QSpanCollection::Span span = d->span(r, c); + if (span.width() > 1 || span.height() > 1) + visualColumn = d->visualColumn(d->columnSpanEndLogical(span.left(), span.width())); + } + while (visualColumn <= right) { ++visualColumn; - if (visualRow == bottom) + r = d->logicalRow(visualRow); + c = d->logicalColumn(visualColumn); + if (r == -1 || c == -1 || (!isRowHidden(r) && !isColumnHidden(c) && d->isCellEnabled(r, c))) + break; + if (wrapped && (originalRow > visualRow || (originalRow == visualRow && originalColumn >= visualColumn))) { + looped = true; + break; + } + } + if (cursorAction == MoveRight || visualColumn <= right) + break; + visualColumn = -1; + if (visualRow == bottom) { + wrapped = true; visualRow = 0; - else - ++visualRow; - while (visualRow < bottom && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn)) + } else { ++visualRow; - break; - } // else MoveRight - case MoveRight: - if (d->hasSpans()) { - QSpanCollection::Span span = d->span(current.row(), current.column()); - visualColumn = d->visualColumn(d->columnSpanEndLogical(span.left(), span.width())); - } - ++visualColumn; - while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn)) - ++visualColumn; - if (d->hasSpans()) { - int column = d->logicalColumn(visualColumn); - QSpanCollection::Span span = d->span(current.row(), column); - visualRow = d->visualRow(span.top()); - } + } + } while (!looped); + if (visualColumn > right) + visualColumn = originalColumn; break; + } case MoveHome: visualColumn = 0; while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn)) @@ -1613,14 +1675,15 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi return d->model->index(newRow, current.column(), d->root); }} + d->visualCursor = QPoint(visualColumn, visualRow); int logicalRow = d->logicalRow(visualRow); int logicalColumn = d->logicalColumn(visualColumn); if (!d->model->hasIndex(logicalRow, logicalColumn, d->root)) return QModelIndex(); QModelIndex result = d->model->index(logicalRow, logicalColumn, d->root); - if (!isIndexHidden(result) && d->isIndexEnabled(result)) - return d->model->index(logicalRow, logicalColumn, d->root); + if (!d->isRowHidden(logicalRow) && !d->isColumnHidden(logicalColumn) && d->isIndexEnabled(result)) + return result; return QModelIndex(); } @@ -2375,7 +2438,8 @@ bool QTableView::isCornerButtonEnabled() const QRect QTableView::visualRect(const QModelIndex &index) const { Q_D(const QTableView); - if (!d->isIndexValid(index) || index.parent() != d->root || isIndexHidden(index) ) + if (!d->isIndexValid(index) || index.parent() != d->root + || (!d->hasSpans() && isIndexHidden(index))) return QRect(); d->executePostedLayout(); diff --git a/src/gui/itemviews/qtableview_p.h b/src/gui/itemviews/qtableview_p.h index c785bd7..9fa14c2 100644 --- a/src/gui/itemviews/qtableview_p.h +++ b/src/gui/itemviews/qtableview_p.h @@ -135,7 +135,8 @@ public: rowSectionAnchor(-1), columnSectionAnchor(-1), columnResizeTimerID(0), rowResizeTimerID(0), horizontalHeader(0), verticalHeader(0), - sortingEnabled(false), geometryRecursionBlock(false) + sortingEnabled(false), geometryRecursionBlock(false), + visualCursor(QPoint()) { wrapItemText = true; #ifndef QT_NO_DRAGANDDROP @@ -183,6 +184,7 @@ public: QWidget *cornerWidget; bool sortingEnabled; bool geometryRecursionBlock; + QPoint visualCursor; // (Row,column) cell coordinates to track through span navigation. QSpanCollection spans; diff --git a/tests/auto/qtableview/tst_qtableview.cpp b/tests/auto/qtableview/tst_qtableview.cpp index bb0e226..4ea1b19 100644 --- a/tests/auto/qtableview/tst_qtableview.cpp +++ b/tests/auto/qtableview/tst_qtableview.cpp @@ -100,6 +100,9 @@ private slots: void moveCursor_data(); void moveCursor(); + void moveCursorStrikesBack_data(); + void moveCursorStrikesBack(); + void hideRows_data(); void hideRows(); @@ -252,12 +255,43 @@ public: row_count(rows), column_count(columns), can_fetch_more(false), - fetch_more_count(0) {} + fetch_more_count(0), + disabled_rows(), + disabled_columns() {} int rowCount(const QModelIndex& = QModelIndex()) const { return row_count; } int columnCount(const QModelIndex& = QModelIndex()) const { return column_count; } bool isEditable(const QModelIndex &) const { return true; } + Qt::ItemFlags flags(const QModelIndex &index) const + { + Qt::ItemFlags index_flags = QAbstractTableModel::flags(index); + if (disabled_rows.contains(index.row()) + || disabled_columns.contains(index.column())) + index_flags &= ~Qt::ItemIsEnabled; + return index_flags; + } + + void disableRow(int row) + { + disabled_rows.insert(row); + } + + void enableRow(int row) + { + disabled_rows.remove(row); + } + + void disableColumn(int column) + { + disabled_columns.insert(column); + } + + void enableColumn(int column) + { + disabled_columns.remove(column); + } + QVariant data(const QModelIndex &idx, int role) const { if (!idx.isValid() || idx.row() >= row_count || idx.column() >= column_count) { @@ -363,6 +397,8 @@ public: int column_count; bool can_fetch_more; int fetch_more_count; + QSet disabled_rows; + QSet disabled_columns; }; class QtTestTableView : public QTableView @@ -834,7 +870,7 @@ void tst_QTableView::moveCursor_data() << 4 << 4 << -1 << 2 << 0 << 2 << int(QtTestTableView::MoveNext) << int(Qt::NoModifier) - << 1 << 0 << IntPair(0,0) << IntPair(3,0); + << 1 << 3 << IntPair(0,0) << IntPair(3,0); // MoveLeft QTest::newRow("MoveLeft (0,0)") @@ -1137,6 +1173,132 @@ void tst_QTableView::moveCursor() QCOMPARE(newIndex.column(), expectedColumn); } +void tst_QTableView::moveCursorStrikesBack_data() +{ + QTest::addColumn("hideRow"); + QTest::addColumn("hideColumn"); + QTest::addColumn("disableRows"); + QTest::addColumn("disableColumns"); + QTest::addColumn("span"); + + QTest::addColumn("startRow"); + QTest::addColumn("startColumn"); + QTest::addColumn("cursorMoveActions"); + QTest::addColumn("expectedRow"); + QTest::addColumn("expectedColumn"); + + QTest::newRow("Last column disabled. Task QTBUG-3878") << -1 << -1 + << IntList() + << (IntList() << 6) + << QRect() + << 0 << 5 << (IntList() << int(QtTestTableView::MoveNext)) + << 1 << 0; + + QTest::newRow("Span, anchor column hidden") << -1 << 1 + << IntList() + << IntList() + << QRect(1, 2, 2, 3) + << 2 << 0 << (IntList() << int(QtTestTableView::MoveNext)) + << 2 << 2; + + QTest::newRow("Span, anchor column disabled") << -1 << -1 + << IntList() + << (IntList() << 1) + << QRect(1, 2, 2, 3) + << 2 << 0 << (IntList() << int(QtTestTableView::MoveNext)) + << 2 << 2; + + QTest::newRow("Span, anchor row hidden") << 2 << -1 + << IntList() + << IntList() + << QRect(1, 2, 2, 3) + << 1 << 2 << (IntList() << int(QtTestTableView::MoveDown)) + << 3 << 2; + + QTest::newRow("Span, anchor row disabled") << -1 << -1 + << (IntList() << 2) + << IntList() + << QRect(1, 2, 2, 3) + << 1 << 2 << (IntList() << int(QtTestTableView::MoveDown)) + << 3 << 2; + + QTest::newRow("Move through span right") << -1 << -1 + << IntList() + << IntList() + << QRect(1, 2, 2, 3) + << 3 << 0 << (IntList() << int(QtTestTableView::MoveRight) << int(QtTestTableView::MoveRight)) + << 3 << 3; + + QTest::newRow("Move through span left") << -1 << -1 + << IntList() + << IntList() + << QRect(1, 2, 2, 3) + << 3 << 3 << (IntList() << int(QtTestTableView::MoveLeft) << int(QtTestTableView::MoveLeft)) + << 3 << 0; + + QTest::newRow("Move through span down") << -1 << -1 + << IntList() + << IntList() + << QRect(1, 2, 2, 3) + << 1 << 2 << (IntList() << int(QtTestTableView::MoveDown) << int(QtTestTableView::MoveDown)) + << 5 << 2; + + QTest::newRow("Move through span up") << -1 << -1 + << IntList() + << IntList() + << QRect(1, 2, 2, 3) + << 5 << 2 << (IntList() << int(QtTestTableView::MoveUp) << int(QtTestTableView::MoveUp)) + << 1 << 2; +} + +void tst_QTableView::moveCursorStrikesBack() +{ + QFETCH(int, hideRow); + QFETCH(int, hideColumn); + QFETCH(IntList, disableRows); + QFETCH(IntList, disableColumns); + QFETCH(QRect, span); + + QFETCH(int, startRow); + QFETCH(int, startColumn); + QFETCH(IntList, cursorMoveActions); + QFETCH(int, expectedRow); + QFETCH(int, expectedColumn); + + QtTestTableModel model(7, 7); + QtTestTableView view; + view.setModel(&model); + view.hideRow(hideRow); + view.hideColumn(hideColumn); + + foreach (int row, disableRows) + model.disableRow(row); + foreach (int column, disableColumns) + model.disableColumn(column); + + if (span.height() && span.width()) + view.setSpan(span.top(), span.left(), span.height(), span.width()); + view.show(); + + QModelIndex index = model.index(startRow, startColumn); + view.setCurrentIndex(index); + + int newRow = -1; + int newColumn = -1; + foreach (int cursorMoveAction, cursorMoveActions) { + QModelIndex newIndex = view.moveCursor((QtTestTableView::CursorAction)cursorMoveAction, 0); + view.setCurrentIndex(newIndex); + newRow = newIndex.row(); + newColumn = newIndex.column(); + } + + // expected fails, task 119433 + if(newRow == -1) + return; + QCOMPARE(newRow, expectedRow); + QCOMPARE(newColumn, expectedColumn); +} + void tst_QTableView::hideRows_data() { QTest::addColumn("rowCount"); -- cgit v0.12