TableView: fall back to use implicit size for delegate items

Rather than forcing users to set TableView.cellWidth/cellHeight (and
therefore also force them to create an attached object for every
cell), we now also accept setting implict size as a fall back.

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

5 files changed, 145 insertions, 37 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 9080ccad18..65fb80a29a 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -489,15 +489,41 @@ Qt::Edge QQuickTableViewPrivate::nextEdgeToUnload(const QRectF rect)
     return Qt::Edge(0);
 }
 
+qreal QQuickTableViewPrivate::cellWidth(const QPoint& cell)
+{
+    // If a delegate item has TableView.cellWidth set, then
+    // we prefer that. Otherwise we fall back to use implicitWidth.
+    // Using an items width directly is not an option, since we change
+    // it during layout (which would also cause problems when recycling items).
+    auto const cellItem = loadedTableItem(cell)->item;
+    if (auto const attached = getAttachedObject(cellItem)) {
+        if (!attached->m_cellWidth.isNull)
+            return attached->m_cellWidth;
+    }
+    return cellItem->implicitWidth();
+}
+
+qreal QQuickTableViewPrivate::cellHeight(const QPoint& cell)
+{
+    // If a delegate item has TableView.cellHeight set, then
+    // we prefer that. Otherwise we fall back to use implicitHeight.
+    // Using an items height directly is not an option, since we change
+    // it during layout (which would also cause problems when recycling items).
+    auto const cellItem = loadedTableItem(cell)->item;
+    if (auto const attached = getAttachedObject(cellItem)) {
+        if (!attached->m_cellHeight.isNull)
+            return attached->m_cellHeight;
+    }
+    return cellItem->implicitHeight();
+}
+
 void QQuickTableViewPrivate::calculateColumnWidthsAfterRebuilding()
 {
     qreal prevColumnWidth = 0;
     for (int column = loadedTable.left(); column <= loadedTable.right(); ++column) {
         qreal columnWidth = 0;
-        for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row) {
-            if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
-                columnWidth = qMax(columnWidth, attached->cellWidth());
-        }
+        for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row)
+            columnWidth = qMax(columnWidth, cellWidth(QPoint(column, row)));
 
         if (columnWidth <= 0)
             columnWidth = kDefaultColumnWidth;
@@ -521,10 +547,8 @@ void QQuickTableViewPrivate::calculateRowHeightsAfterRebuilding()
     qreal prevRowHeight = 0;
     for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row) {
         qreal rowHeight = 0;
-        for (int column = loadedTable.left(); column <= loadedTable.right(); ++column) {
-            if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
-                rowHeight = qMax(rowHeight, attached->cellHeight());
-        }
+        for (int column = loadedTable.left(); column <= loadedTable.right(); ++column)
+            rowHeight = qMax(rowHeight, cellHeight(QPoint(column, row)));
 
         if (rowHeight <= 0)
             rowHeight = kDefaultRowHeight;
@@ -552,10 +576,8 @@ void QQuickTableViewPrivate::calculateColumnWidth(int column)
     }
 
     qreal columnWidth = 0;
-    for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row) {
-        if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
-            columnWidth = qMax(columnWidth, attached->cellWidth());
-    }
+    for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row)
+        columnWidth = qMax(columnWidth, cellWidth(QPoint(column, row)));
 
     if (columnWidth <= 0)
         columnWidth = kDefaultColumnWidth;
@@ -575,10 +597,8 @@ void QQuickTableViewPrivate::calculateRowHeight(int row)
     }
 
     qreal rowHeight = 0;
-    for (int column = loadedTable.left(); column <= loadedTable.right(); ++column) {
-        if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
-            rowHeight = qMax(rowHeight, attached->cellHeight());
-    }
+    for (int column = loadedTable.left(); column <= loadedTable.right(); ++column)
+        rowHeight = qMax(rowHeight, cellHeight(QPoint(column, row)));
 
     if (rowHeight <= 0)
         rowHeight = kDefaultRowHeight;
@@ -661,10 +681,8 @@ qreal QQuickTableViewPrivate::columnWidth(int column)
     }
 
     // If we have an item loaded at column, return the width of the item.
-    if (column >= loadedTable.left() && column <= loadedTable.right()) {
-        if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, 0))->item))
-            return attached->cellWidth();
-    }
+    if (column >= loadedTable.left() && column <= loadedTable.right())
+        return cellWidth(QPoint(column, loadedTable.top()));
 
     return -1;
 }
@@ -697,10 +715,8 @@ qreal QQuickTableViewPrivate::rowHeight(int row)
     }
 
     // If we have an item loaded at row, return the height of the item.
-    if (row >= loadedTable.top() && row <= loadedTable.bottom()) {
-        if (auto attached = getAttachedObject(loadedTableItem(QPoint(0, row))->item))
-            return attached->cellHeight();
-    }
+    if (row >= loadedTable.top() && row <= loadedTable.bottom())
+        return cellWidth(QPoint(loadedTable.left(), row));
 
     return -1;
 }
@@ -848,12 +864,8 @@ void QQuickTableViewPrivate::layoutTopLeftItem()
     auto topLeftItem = loadedTableItem(cell);
     auto item = topLeftItem->item;
 
-    qreal width = 0;
-    qreal height = 0;
-    if (auto attached = getAttachedObject(loadedTableItem(cell)->item)) {
-        width = attached->cellWidth();
-        height = attached->cellHeight();
-    }
+    qreal width = cellWidth(cell);
+    qreal height = cellHeight(cell);
     if (width <= 0)
         width = kDefaultColumnWidth;
     if (height <= 0)
diff --git a/src/quick/items/qquicktableview_p.h b/src/quick/items/qquicktableview_p.h
index 0589d83f50..f3a589b9e4 100644
--- a/src/quick/items/qquicktableview_p.h
+++ b/src/quick/items/qquicktableview_p.h
@@ -54,6 +54,7 @@
 #include <QtCore/qpointer.h>
 #include <QtQuick/private/qtquickglobal_p.h>
 #include <QtQuick/private/qquickflickable_p.h>
+#include <QtQml/private/qqmlnullablevalue_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -158,19 +159,19 @@ public:
         Q_EMIT tableViewChanged();
     }
 
-    qreal cellWidth() const { return m_cellSize.width(); }
+    qreal cellWidth() const { return m_cellWidth; }
     void setCellWidth(qreal newWidth) {
-        if (newWidth == m_cellSize.width())
+        if (newWidth == m_cellWidth)
             return;
-        m_cellSize.setWidth(newWidth);
+        m_cellWidth = newWidth;
         Q_EMIT cellWidthChanged();
     }
 
-    qreal cellHeight() const { return m_cellSize.height(); }
+    qreal cellHeight() const { return m_cellHeight; }
     void setCellHeight(qreal newHeight) {
-        if (newHeight == m_cellSize.height())
+        if (newHeight == m_cellHeight)
             return;
-        m_cellSize.setHeight(newHeight);
+        m_cellHeight = newHeight;
         Q_EMIT cellHeightChanged();
     }
 
@@ -201,7 +202,10 @@ private:
     QPointer<QQuickTableView> m_tableview;
     int m_row = -1;
     int m_column = -1;
-    QSizeF m_cellSize;
+    QQmlNullableValue<qreal> m_cellWidth;
+    QQmlNullableValue<qreal> m_cellHeight;
+
+    friend class QQuickTableViewPrivate;
 };
 
 QT_END_NAMESPACE
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index ada7f1f83b..a7c512f063 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -268,6 +268,9 @@ public:
     Qt::Edge nextEdgeToLoad(const QRectF rect);
     Qt::Edge nextEdgeToUnload(const QRectF rect);
 
+    qreal cellWidth(const QPoint &cell);
+    qreal cellHeight(const QPoint &cell);
+
     FxTableItem *loadedTableItem(const QPoint &cell) const;
     FxTableItem *itemNextTo(const FxTableItem *fxTableItem, const QPoint &direction) const;
     FxTableItem *createFxTableItem(const QPoint &cell, QQmlIncubator::IncubationMode incubationMode);
diff --git a/tests/auto/quick/qquicktableview/data/tableviewimplicitsize.qml b/tests/auto/quick/qquicktableview/data/tableviewimplicitsize.qml
new file mode 100644
index 0000000000..65e4d0861c
--- /dev/null
+++ b/tests/auto/quick/qquicktableview/data/tableviewimplicitsize.qml
@@ -0,0 +1,66 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.12
+import QtQuick.Window 2.3
+import Qt.labs.tableview 1.0
+
+Item {
+    width: 640
+    height: 450
+
+    property alias tableView: tableView
+
+    TableView {
+        id: tableView
+        width: 600
+        height: 400
+        anchors.margins: 1
+        clip: true
+        delegate: tableViewDelegate
+        cacheBuffer: 0
+        columnSpacing: 1
+        rowSpacing: 1
+    }
+
+    Component {
+        id: tableViewDelegate
+        Rectangle {
+            objectName: "tableViewDelegate"
+            color: "lightgray"
+            border.width: 1
+            implicitWidth: 90
+            implicitHeight: 60
+            Text {
+                anchors.centerIn: parent
+                text: modelData
+            }
+        }
+    }
+
+}
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 16b3acab99..c12e9a9432 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -86,6 +86,7 @@ private slots:
     void emptyModel_data();
     void emptyModel();
     void checkZeroSizedDelegate();
+    void checkImplicitSizeDelegate();
     void noDelegate();
     void countDelegateItems_data();
     void countDelegateItems();
@@ -209,6 +210,28 @@ void tst_QQuickTableView::checkZeroSizedDelegate()
     }
 }
 
+void tst_QQuickTableView::checkImplicitSizeDelegate()
+{
+    // Check that we can set the size of delegate items using
+    // implicit width/height, instead of forcing the user to
+    // create an attached object by using TableView.cellWidth/Height.
+    LOAD_TABLEVIEW("tableviewimplicitsize.qml");
+
+    auto model = TestModelAsVariant(100, 100);
+    tableView->setModel(model);
+
+    WAIT_UNTIL_POLISHED;
+
+    auto items = tableViewPrivate->loadedItems;
+    QVERIFY(!items.isEmpty());
+
+    for (auto fxItem : tableViewPrivate->loadedItems) {
+        auto item = fxItem->item;
+        QCOMPARE(item->width(), 90);
+        QCOMPARE(item->height(), 60);
+    }
+}
+
 void tst_QQuickTableView::noDelegate()
 {
     // Check that you can skip setting a delegate without