From 7adfb01943b8681a51d6c01fed6d06b864f6d010 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Mon, 25 Mar 2024 10:02:57 +0100 Subject: [PATCH] SelectionRectangle: ensure we remove or update selection handles if selection changes If you make a selection with SelectionRectangle and then resize a row, the selection handles will not move to the resized position. Likewise, if you do a selection, but then clear the selection by interacting with the ItemSelectionModel directly, the selection handles will not be updated to reflect the changes. To fix this bug, this patch will add a callback from TableView to SelectionRectangle that can be used to notify when changes are done outside of SelectionRectangle. This especially allows us to: - Remove the selection handles if the active selection is cleared or changed programatically (or anyway not by SelectionRectangle). - Keep the selection handles, and update their position, when rows or columns are merely resized. This change also allows us to clean up qquickselectionrectangle.cpp a bit, and remove e.g a now superfluous tap handler listener. Task-number: QTBUG-121143 Pick-to: 6.6 6.5 Change-Id: Id170520d49bc92c0bb9d16deaba741cab6f5c553 Reviewed-by: Santhosh Kumar (cherry picked from commit f0fbedbe69532d9f5d1bc622f0b5b1ed16f23f2b) Reviewed-by: Qt Cherry-pick Bot --- src/quick/items/qquickselectable_p.h | 7 +++ src/quick/items/qquicktableview.cpp | 38 +++++++++++--- src/quick/items/qquicktableview_p_p.h | 4 ++ .../qquickselectionrectangle.cpp | 40 +++++++++------ .../controls/data/tst_selectionrectangle.qml | 49 +++++++++++++++++++ .../tableview/abstracttablemodel/main.qml | 4 ++ 6 files changed, 119 insertions(+), 23 deletions(-) diff --git a/src/quick/items/qquickselectable_p.h b/src/quick/items/qquickselectable_p.h index 726352719f..c7fa5f1809 100644 --- a/src/quick/items/qquickselectable_p.h +++ b/src/quick/items/qquickselectable_p.h @@ -23,6 +23,11 @@ QT_BEGIN_NAMESPACE class Q_QUICK_PRIVATE_EXPORT QQuickSelectable { public: + enum class CallBackFlag { + CancelSelection, + SelectionRectangleChanged + }; + virtual QQuickItem *selectionPointerHandlerTarget() const = 0; virtual bool startSelection(const QPointF &pos) = 0; @@ -33,6 +38,8 @@ public: virtual QRectF selectionRectangle() const = 0; virtual QSizeF scrollTowardsSelectionPoint(const QPointF &pos, const QSizeF &step) = 0; + + virtual void setCallback(std::function func) = 0; }; QT_END_NAMESPACE diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp index 93b6a42a6d..ac58a7a389 100644 --- a/src/quick/items/qquicktableview.cpp +++ b/src/quick/items/qquicktableview.cpp @@ -1698,6 +1698,7 @@ void QQuickTableViewPrivate::setSelectionStartPos(const QPointF &pos) return; // Update selection model + QScopedValueRollback callbackGuard(inSelectionModelUpdate, true); updateSelection(prevSelection, selection()); } @@ -1746,6 +1747,7 @@ void QQuickTableViewPrivate::setSelectionEndPos(const QPointF &pos) return; // Update selection model + QScopedValueRollback callbackGuard(inSelectionModelUpdate, true); updateSelection(prevSelection, selection()); } @@ -1814,14 +1816,22 @@ void QQuickTableViewPrivate::updateSelection(const QRect &oldSelection, const QR selectionModel->select(select, QItemSelectionModel::Select); } -void QQuickTableViewPrivate::clearSelection() +void QQuickTableViewPrivate::cancelSelectionTracking() { + // Cancel any ongoing key/mouse aided selection tracking selectionStartCell = QPoint(-1, -1); selectionEndCell = QPoint(-1, -1); existingSelection.clear(); + if (selectableCallbackFunction) + selectableCallbackFunction(QQuickSelectable::CallBackFlag::CancelSelection); +} - if (selectionModel) - selectionModel->clearSelection(); +void QQuickTableViewPrivate::clearSelection() +{ + if (!selectionModel) + return; + QScopedValueRollback callbackGuard(inSelectionModelUpdate, true); + selectionModel->clearSelection(); } void QQuickTableViewPrivate::normalizeSelection() @@ -1959,6 +1969,11 @@ QSizeF QQuickTableViewPrivate::scrollTowardsSelectionPoint(const QPointF &pos, c return dist; } +void QQuickTableViewPrivate::setCallback(std::function func) +{ + selectableCallbackFunction = func; +} + QQuickTableViewAttached *QQuickTableViewPrivate::getAttachedObject(const QObject *object) const { QObject *attachedObject = qmlAttachedPropertiesObject(object); @@ -4065,10 +4080,11 @@ bool QQuickTableViewPrivate::currentInSelectionModel(const QPoint &cell) const void QQuickTableViewPrivate::selectionChangedInSelectionModel(const QItemSelection &selected, const QItemSelection &deselected) { - if (!selectionModel->hasSelection()) { - // Ensure that we cancel any ongoing key/mouse-based selections - // if selectionModel.clearSelection() is called. - clearSelection(); + if (!inSelectionModelUpdate) { + // The selection model was manipulated outside of TableView + // and SelectionRectangle. In that case we cancel any ongoing + // selection tracking. + cancelSelectionTracking(); } const auto &selectedIndexes = selected.indexes(); @@ -4900,8 +4916,10 @@ void QQuickTableViewPrivate::handleTap(const QQuickHandlerPoint &point) // the current selection and move the current index instead. if (pointerNavigationEnabled) { q->closeEditor(); - if (selectionBehavior != QQuickTableView::SelectionDisabled) + if (selectionBehavior != QQuickTableView::SelectionDisabled) { clearSelection(); + cancelSelectionTracking(); + } setCurrentIndexFromTap(point.position()); } } @@ -5040,6 +5058,8 @@ bool QQuickTableViewPrivate::setCurrentIndexFromKeyEvent(QKeyEvent *e) if (loadedItems.contains(serializedStartIndex)) { const QRectF startGeometry = loadedItems.value(serializedStartIndex)->geometry(); setSelectionStartPos(startGeometry.center()); + if (selectableCallbackFunction) + selectableCallbackFunction(QQuickSelectable::CallBackFlag::SelectionRectangleChanged); } } }; @@ -5052,6 +5072,8 @@ bool QQuickTableViewPrivate::setCurrentIndexFromKeyEvent(QKeyEvent *e) if (loadedItems.contains(serializedEndIndex)) { const QRectF endGeometry = loadedItems.value(serializedEndIndex)->geometry(); setSelectionEndPos(endGeometry.center()); + if (selectableCallbackFunction) + selectableCallbackFunction(QQuickSelectable::CallBackFlag::SelectionRectangleChanged); } } selectionModel->setCurrentIndex(q->modelIndex(cell), QItemSelectionModel::NoUpdate); diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h index a3768fda71..d2dc33c2f5 100644 --- a/src/quick/items/qquicktableview_p_p.h +++ b/src/quick/items/qquicktableview_p_p.h @@ -360,6 +360,8 @@ public: QPointer selectionModel; QQuickTableView::SelectionBehavior selectionBehavior = QQuickTableView::SelectCells; QQuickTableView::SelectionMode selectionMode = QQuickTableView::ExtendedSelection; + std::function selectableCallbackFunction; + bool inSelectionModelUpdate = false; int assignedPositionViewAtRowAfterRebuild = 0; int assignedPositionViewAtColumnAfterRebuild = 0; @@ -587,6 +589,8 @@ public: void normalizeSelection() override; QRectF selectionRectangle() const override; QSizeF scrollTowardsSelectionPoint(const QPointF &pos, const QSizeF &step) override; + void setCallback(std::function func) override; + void cancelSelectionTracking(); QPoint clampedCellAtPos(const QPointF &pos) const; virtual void updateSelection(const QRect &oldSelection, const QRect &newSelection); diff --git a/src/quicktemplates/qquickselectionrectangle.cpp b/src/quicktemplates/qquickselectionrectangle.cpp index 7ef9996aae..fb737d017a 100644 --- a/src/quicktemplates/qquickselectionrectangle.cpp +++ b/src/quicktemplates/qquickselectionrectangle.cpp @@ -176,14 +176,6 @@ QQuickSelectionRectanglePrivate::QQuickSelectionRectanglePrivate() m_scrollSpeed = QSizeF(qAbs(dist.width() * 0.007), qAbs(dist.height() * 0.007)); }); - QObject::connect(m_tapHandler, &QQuickTapHandler::tapped, [this] { - const auto modifiers = m_tapHandler->point().modifiers(); - if (modifiers != Qt::NoModifier) - return; - - updateActiveState(false); - }); - QObject::connect(m_tapHandler, &QQuickTapHandler::pressedChanged, [this]() { if (!m_tapHandler->isPressed()) return; @@ -223,9 +215,6 @@ QQuickSelectionRectanglePrivate::QQuickSelectionRectanglePrivate() m_selectable->setSelectionEndPos(pos); updateHandles(); updateActiveState(true); - } else if (modifiers == Qt::NoModifier) { - // Don't select any cell - updateActiveState(false); } }); @@ -284,10 +273,11 @@ QQuickSelectionRectanglePrivate::QQuickSelectionRectanglePrivate() return; if (m_dragHandler->active()) { - // Start a new selection if no selection exists from before. - // Note that if the user pressed ControlModifier while starting - // the drag, the first cell will be selected already on press. - if (!m_active) { + // Start a new selection unless there is an active selection + // already, and one of the relevant modifiers are being held. + // In that case we continue to extend the active selection instead. + const bool modifiersHeld = modifiers & (Qt::ControlModifier | Qt::ShiftModifier); + if (!m_active || !modifiersHeld) { if (!m_selectable->startSelection(startPos)) return; m_selectable->setSelectionStartPos(startPos); @@ -481,6 +471,25 @@ void QQuickSelectionRectanglePrivate::connectToTarget() if (const auto flickable = qobject_cast(m_target)) { connect(flickable, &QQuickFlickable::interactiveChanged, this, &QQuickSelectionRectanglePrivate::updateSelectionMode); } + + // Add a callback function that tells if the selection was + // modified outside of the actions taken by SelectionRectangle. + m_selectable->setCallback([this](QQuickSelectable::CallBackFlag flag){ + switch (flag) { + case QQuickSelectable::CallBackFlag::CancelSelection: + // The selection is either cleared, or can no longer be + // represented as a rectangle with two selection handles. + updateActiveState(false); + break; + case QQuickSelectable::CallBackFlag::SelectionRectangleChanged: + // The selection has changed, but the selection is still + // rectangular and without holes. + updateHandles(); + break; + default: + Q_UNREACHABLE(); + } + }); } void QQuickSelectionRectanglePrivate::updateSelectionMode() @@ -559,6 +568,7 @@ void QQuickSelectionRectangle::setTarget(QQuickItem *target) d->m_tapHandler->setParent(this); d->m_dragHandler->setParent(this); d->m_target->disconnect(this); + d->m_selectable->setCallback(nullptr); } d->m_target = target; diff --git a/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml b/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml index 44a6c558ab..302851db4a 100644 --- a/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml +++ b/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml @@ -727,4 +727,53 @@ TestCase { } } } + + function test_programmatic_unselect() { + // Check that the SelectionRectangle will be deactivated if the + // selection is changed programatically. + let tableView = createTemporaryObject(tableviewComp, testCase) + verify(tableView) + let selectionRectangle = tableView.selectionRectangle + verify(selectionRectangle) + + selectionRectangle.selectionMode = SelectionRectangle.Drag + + verify(!tableView.selectionModel.hasSelection) + mouseDrag(tableView, 1, 1, (cellWidth * 2) - 2, 1, Qt.LeftButton) + compare(tableView.selectionModel.selectedIndexes.length, 2) + verify(selectionRectangle.active) + + tableView.selectionModel.clearSelection() + verify(!selectionRectangle.active) + } + + function test_extend_using_keyboard() { + // Check that the bottom-right selection handle will move if an + // acitve selection is extended with the keyboard + let tableView = createTemporaryObject(tableviewComp, testCase) + verify(tableView) + let selectionRectangle = tableView.selectionRectangle + verify(selectionRectangle) + + selectionRectangle.bottomRightHandle = bottomRightHandleComp + selectionRectangle.selectionMode = SelectionRectangle.Drag + + tableView.forceActiveFocus() + verify(!tableView.selectionModel.hasSelection) + mouseDrag(tableView, 1, 1, (cellWidth * 2) - 2, 1, Qt.LeftButton) + compare(tableView.selectionModel.selectedIndexes.length, 2) + verify(selectionRectangle.active) + verify(bottomRightHandle) + compare(bottomRightHandle.x, (cellWidth * 2) - (bottomRightHandle.width / 2)) + compare(bottomRightHandle.y, cellHeight - (bottomRightHandle.height / 2)) + + keyPress(Qt.Key_Down, Qt.ShiftModifier) + keyRelease(Qt.Key_Down, Qt.ShiftModifier) + keyPress(Qt.Key_Right, Qt.ShiftModifier) + keyRelease(Qt.Key_Right, Qt.ShiftModifier) + verify(selectionRectangle.active) + compare(tableView.selectionModel.selectedIndexes.length, 6) + compare(bottomRightHandle.x, (cellWidth * 3) - (bottomRightHandle.width / 2)) + compare(bottomRightHandle.y, (cellHeight * 2) - (bottomRightHandle.height / 2)) + } } diff --git a/tests/manual/tableview/abstracttablemodel/main.qml b/tests/manual/tableview/abstracttablemodel/main.qml index e8d1192ede..5578e86dba 100644 --- a/tests/manual/tableview/abstracttablemodel/main.qml +++ b/tests/manual/tableview/abstracttablemodel/main.qml @@ -198,6 +198,10 @@ ApplicationWindow { ] Layout.fillWidth: false } + Button { + text: "Clear selection" + onClicked: tableView.selectionModel.clearSelection() + } } }