From efdaba37d61e82f27a7bcbb49e176313452f59f9 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Wed, 2 Jun 2021 15:24:25 +0200 Subject: [PATCH] QAItemView: in MultiSelection, press deselects only if no drag can start In MultiSelection mode, items are by default toggled on press, which follows the example of standard Windows controls. However, when dragging is enabled, then the press might be the beginning of a drag'n'drop operation, and deselecting the item on press breaks the selection and user experience. Don't toggle the selection for presses on an already selected item that might get dragged; instead, wait for the release event. Extend the test case slightly to cover the special case. Dragging a selection in a drag-enabled and MultiSelection item view wasn't possible before either. Fixes: QTBUG-59888 Change-Id: Ibd3e95a71ea63dd1e9bc3c8a723eafa9a1c21afa Reviewed-by: Lars Knoll Reviewed-by: David Skoland --- src/widgets/itemviews/qabstractitemview.cpp | 13 +++++++-- .../tst_qabstractitemview.cpp | 29 ++++++++++++------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index f4f41e6f68f..8a923e44a76 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -3981,16 +3981,23 @@ QItemSelectionModel::SelectionFlags QAbstractItemViewPrivate::multiSelectionComm return QItemSelectionModel::Toggle|selectionBehaviorFlags(); break; case QEvent::MouseButtonPress: - if (static_cast(event)->button() == Qt::LeftButton) - return QItemSelectionModel::Toggle|selectionBehaviorFlags(); // toggle + if (static_cast(event)->button() == Qt::LeftButton) { + // since the press might start a drag, deselect only on release + if (!pressedAlreadySelected || !dragEnabled || !isIndexDragEnabled(index)) + return QItemSelectionModel::Toggle|selectionBehaviorFlags(); // toggle + } break; case QEvent::MouseButtonRelease: - if (static_cast(event)->button() == Qt::LeftButton) + if (static_cast(event)->button() == Qt::LeftButton) { + if (pressedAlreadySelected && dragEnabled && isIndexDragEnabled(index) && index == pressedIndex) + return QItemSelectionModel::Toggle|selectionBehaviorFlags(); return QItemSelectionModel::NoUpdate|selectionBehaviorFlags(); // finalize + } break; case QEvent::MouseMove: if (static_cast(event)->buttons() & Qt::LeftButton) return QItemSelectionModel::ToggleCurrent|selectionBehaviorFlags(); // toggle drag select + break; default: break; } diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index e7637f80137..69fac4bd963 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -2790,9 +2790,14 @@ void tst_QAbstractItemView::mouseSelection_data() << QList{SelectionEvent(SelectionEvent::Press, 1), SelectionEvent(SelectionEvent::Press, 1)} << QList{}; - QTest::addRow("Multi:Click") << QAbstractItemView::MultiSelection << false - << QList{SelectionEvent(SelectionEvent::Click, 2)} - << QList{2}; + QTest::addRow("Multi:Press twice with drag enabled") << QAbstractItemView::MultiSelection << true + << QList{SelectionEvent(SelectionEvent::Click, 1), + SelectionEvent(SelectionEvent::Press, 1)} + << QList{1}; + QTest::addRow("Multi:Press and click with drag enabled") << QAbstractItemView::MultiSelection << true + << QList{SelectionEvent(SelectionEvent::Press, 1), + SelectionEvent(SelectionEvent::Click, 1)} + << QList{}; QTest::addRow("Multi:Press,Press") << QAbstractItemView::MultiSelection << false << QList{SelectionEvent(SelectionEvent::Press, 2), SelectionEvent(SelectionEvent::Press, 3)} @@ -2828,11 +2833,15 @@ void tst_QAbstractItemView::mouseSelection_data() << QList{3, 4}; // Multi: Press-dragging a selection should not deselect #QTBUG-59888 QTest::addRow("Multi:Press-Drag selection") << QAbstractItemView::MultiSelection << true - << QList{SelectionEvent(SelectionEvent::Press, 2), - SelectionEvent(SelectionEvent::Move, 5), - SelectionEvent(SelectionEvent::Release), + // with drag'n'drop enabled, we cannot drag a selection + << QList{SelectionEvent(SelectionEvent::Click, 2), + SelectionEvent(SelectionEvent::Click, 3), + SelectionEvent(SelectionEvent::Click, 4), + SelectionEvent(SelectionEvent::Click, 5), SelectionEvent(SelectionEvent::Press, 3), - SelectionEvent(SelectionEvent::Move, 5)} + // two moves needed because of distance and state logic in QAbstractItemView + SelectionEvent(SelectionEvent::Move, 5), + SelectionEvent(SelectionEvent::Move, 6)} << QList{2, 3, 4, 5}; // Extended selection: Press selects a single item @@ -2962,12 +2971,12 @@ void tst_QAbstractItemView::mouseSelection() } QList actualSelected; - const auto selectedIndexes = dragEnabled ? dragRecorder->draggedIndexes - : view->selectionModel()->selectedIndexes(); + const auto selectedIndexes = dragRecorder->dragStarted + ? dragRecorder->draggedIndexes + : view->selectionModel()->selectedIndexes(); for (auto index : selectedIndexes) actualSelected << index.row(); - QEXPECT_FAIL("Multi:Press-Drag selection", "QTBUG-59889", Continue); QCOMPARE(actualSelected, selectedRows); }