QQQuickTreeView: double expand from TreeView, not TreeViewDelegate

We should ideally implement as much mouse logic as
possible outside the delegate. This will make it
easier to implement custom delegates, since you would
then only have to care about the visuals, and not the
mouse logic.

Therefore, move the doubleTap-to-toggle-expanded
logic out of TreeViewDelegate, and into TreeView, for
all custom delegates to enjoy.

Change-Id: I01f49252d35fed033f162d7bab47478abefe73d1
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

4 files changed, 55 insertions, 5 deletions

diff --git a/src/quick/items/qquicktreeview.cpp b/src/quick/items/qquicktreeview.cpp
index a8df1ac00a..a674d74620 100644
--- a/src/quick/items/qquicktreeview.cpp
+++ b/src/quick/items/qquicktreeview.cpp
@@ -5,6 +5,7 @@
 
 #include <QtCore/qobject.h>
 #include <QtQml/qqmlcontext.h>
+#include <QtQuick/private/qquicktaphandler_p.h>
 
 #include <QtQmlModels/private/qqmltreemodeltotablemodel_p_p.h>
 
@@ -78,6 +79,14 @@
 
     See also \l {Required Properties}.
 
+    By default, TreeView \l {toggleExpanded()}{toggles} the expanded state
+    of a row when you double tap on it. Since this is in conflict with
+    double tapping to edit a cell, TreeView sets \l editTriggers to
+    \c TableView.EditKeyPressed by default (which is different from TableView,
+    which uses \c {TableView.EditKeyPressed | TableView.DoubleTapped}.
+    If you change \l editTriggers to also contain \c TableView.DoubleTapped,
+    toggling the expanded state with a double tap will be disabled.
+
     \note A TreeView only accepts a model that inherits \l QAbstractItemModel.
 */
 
@@ -386,6 +395,7 @@ QQuickTreeView::QQuickTreeView(QQuickItem *parent)
     Q_D(QQuickTreeView);
 
     setSelectionBehavior(SelectRows);
+    setEditTriggers(EditKeyPressed);
 
     // Note: QQuickTableView will only ever see the table model m_treeModelToTableModel, and
     // never the actual tree model that is assigned to us by the application.
@@ -393,6 +403,18 @@ QQuickTreeView::QQuickTreeView(QQuickItem *parent)
     d->QQuickTableViewPrivate::setModelImpl(modelAsVariant);
     QObjectPrivate::connect(&d->m_treeModelToTableModel, &QAbstractItemModel::dataChanged,
                             d, &QQuickTreeViewPrivate::dataChangedCallback);
+
+    auto tapHandler = new QQuickTapHandler(this);
+    tapHandler->setAcceptedModifiers(Qt::NoModifier);
+    connect(tapHandler, &QQuickTapHandler::doubleTapped, [this, tapHandler]{
+        if (!pointerNavigationEnabled())
+            return;
+        if (editTriggers() & DoubleTapped)
+            return;
+
+        const int row = cellAtPosition(tapHandler->point().pressPosition()).y();
+        toggleExpanded(row);
+    });
 }
 
 QQuickTreeView::~QQuickTreeView()
diff --git a/src/quicktemplates/qquicktreeviewdelegate.cpp b/src/quicktemplates/qquicktreeviewdelegate.cpp
index 01558a20db..20339efa49 100644
--- a/src/quicktemplates/qquicktreeviewdelegate.cpp
+++ b/src/quicktemplates/qquicktreeviewdelegate.cpp
@@ -274,7 +274,6 @@ QQuickTreeViewDelegate::QQuickTreeViewDelegate(QQuickItem *parent)
 
     auto tapHandler = new QQuickTapHandler(this);
     tapHandler->setAcceptedModifiers(Qt::NoModifier);
-    QObjectPrivate::connect(tapHandler, &QQuickTapHandler::doubleTapped, d_func(), &QQuickTreeViewDelegatePrivate::toggleExpanded);
     QObjectPrivate::connect(this, &QQuickAbstractButton::indicatorChanged, d, &QQuickTreeViewDelegatePrivate::updateIndicatorPointerHandlers);
 
     // Since we override mousePressEvent to avoid QQuickAbstractButton from blocking
diff --git a/tests/auto/quick/qquicktreeview/tst_qquicktreeview.cpp b/tests/auto/quick/qquicktreeview/tst_qquicktreeview.cpp
index 379ab074a3..67f3f21d80 100644
--- a/tests/auto/quick/qquicktreeview/tst_qquicktreeview.cpp
+++ b/tests/auto/quick/qquicktreeview/tst_qquicktreeview.cpp
@@ -29,6 +29,7 @@ using namespace Qt::StringLiterals;
 #define LOAD_TREEVIEW(fileName) \
     view->setSource(testFileUrl(fileName)); \
     view->show(); \
+    view->requestActivate(); \
     QVERIFY(QTest::qWaitForWindowActive(view)); \
     auto treeView = view->rootObject()->property("treeView").value<QQuickTreeView *>(); \
     QVERIFY(treeView); \
@@ -76,6 +77,7 @@ private slots:
     void updatedModifiedModel();
     void insertRows();
     void toggleExpandedUsingArrowKeys();
+    void expandAndCollapsUsingDoubleClick();
     void selectionBehaviorCells_data();
     void selectionBehaviorCells();
     void selectionBehaviorRows();
@@ -782,6 +784,37 @@ void tst_qquicktreeview::toggleExpandedUsingArrowKeys()
     QCOMPARE(treeView->selectionModel()->currentIndex(), treeView->modelIndex(0, row0));
 }
 
+void tst_qquicktreeview::expandAndCollapsUsingDoubleClick()
+{
+    LOAD_TREEVIEW("normaltreeview.qml");
+    // Check that the view only has one row loaded so far (the root of the tree)
+    QCOMPARE(treeViewPrivate->loadedRows.count(), 1);
+
+    // Expand the root by double clicking on the row
+    const auto item = treeView->itemAtCell(0, 0);
+    QVERIFY(item);
+    const QPoint localPos = QPoint(item->width() / 2, item->height() / 2);
+    const QPoint pos = item->window()->contentItem()->mapFromItem(item, localPos).toPoint();
+    QTest::mouseDClick(item->window(), Qt::LeftButton, Qt::NoModifier, pos);
+
+    // We now expect 5 rows, the root pluss it's 4 children. Since
+    // mouseDClick calls processEvents(), it becomes random at this
+    // point if the view has been polished or not. So use QTRY_COMPARE.
+    QTRY_COMPARE(treeViewPrivate->loadedRows.count(), 5);
+
+    // Collapse the root again
+    QTest::mouseDClick(item->window(), Qt::LeftButton, Qt::NoModifier, pos);
+    QTRY_COMPARE(treeViewPrivate->loadedRows.count(), 1);
+
+    // If edit triggers has DoubleTapped set, we should
+    // start to edit instead of expanding.
+    treeView->setEditTriggers(QQuickTableView::DoubleTapped);
+    QTest::mouseDClick(item->window(), Qt::LeftButton, Qt::NoModifier, pos);
+    if (QQuickTest::qIsPolishScheduled(treeView))
+        QVERIFY(QQuickTest::qWaitForPolish(treeView));
+    QTRY_COMPARE(treeViewPrivate->loadedRows.count(), 1);
+}
+
 void tst_qquicktreeview::selectionBehaviorCells_data()
 {
     QTest::addColumn<QPoint>("startCell");
diff --git a/tests/manual/treeview/sidebyside/data/treeview.qml b/tests/manual/treeview/sidebyside/data/treeview.qml
index 6c51b7c413..7d265c44e1 100644
--- a/tests/manual/treeview/sidebyside/data/treeview.qml
+++ b/tests/manual/treeview/sidebyside/data/treeview.qml
@@ -134,10 +134,6 @@ ApplicationWindow {
             required property int hasChildren
             required property int depth
 
-            TapHandler {
-                onTapped: treeView.toggleExpanded(row)
-            }
-
             Text {
                 id: indicator
                 visible: root.isTreeNode && root.hasChildren