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>

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()

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

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");

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