QQuickTableView: don't override margins API in TableView

Flickable has a margins API with the exact same naming as
the margins API in TableView. This means that overriding those
properties in TableView was an oversight, and a mistake.

This patch will therefore remove the margins API from
TableView. However, since the API already exists is in
Flickable, the resulting API remains unchanged. But it
will ease the TableView implementation a bit, since we
can then remove code that takes margins into account (since
Flickable does this automatically for us).

The only real difference that will take effect from this
change, is that any overlay or underlay items inside the
flickable will need to have negative coordinates if you
want to position them on top of the margins (e.g to create
a header on top of the table).

Change-Id: I43af66e49f5ddff90739a1c789aacb77ed18b4ce
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

1 files changed, 19 insertions, 99 deletions

diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 6a6849eeae..a4c954b1b4 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -111,8 +111,6 @@ private slots:
     void checkLayoutOfEqualSizedDelegateItems();
     void checkFocusRemoved_data();
     void checkFocusRemoved();
-    void checkTableMargins_data();
-    void checkTableMargins();
     void fillTableViewButNothingMore_data();
     void fillTableViewButNothingMore();
     void checkInitialAttachedProperties_data();
@@ -503,31 +501,28 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     // to do the calculations once, and use them for both axis, below.
     QCOMPARE(tableView->width(), tableView->height());
     QCOMPARE(tableView->rowSpacing(), tableView->columnSpacing());
-    QCOMPARE(tableView->leftMargin(), tableView->rightMargin());
-    QCOMPARE(tableView->topMargin(), tableView->bottomMargin());
 
     const int tableSize = 100;
     const int cellSizeSmall = 100;
     const int cellSizeLarge = 200;
     const int spacing = 1;
-    const int margin = 10;
     const int smallCellCount = 20;
     const int largeCellCount = tableSize - smallCellCount;
-    const qreal spacingAndMargins = ((tableSize - 1) * spacing) + (margin * 2);
+    const qreal accumulatedSpacing = ((tableSize - 1) * spacing);
     auto model = TestModelAsVariant(tableSize, tableSize);
 
     tableView->setModel(model);
 
     WAIT_UNTIL_POLISHED;
 
-    const qreal expectedSizeInit = (tableSize * cellSizeSmall) + ((tableSize - 1) * spacing) + (margin * 2);
+    const qreal expectedSizeInit = (tableSize * cellSizeSmall) + ((tableSize - 1) * spacing);
     QCOMPARE(tableView->contentWidth(), expectedSizeInit);
     QCOMPARE(tableView->contentHeight(), expectedSizeInit);
 
     // Flick in 5 more rows and columns, but not so far that we start loading in
     // the ones that are bigger. Loading in more rows and columns of the same
     // size as the initial ones should not change the first prediction.
-    qreal flickTo = ((cellSizeSmall + spacing) * 5) + margin;
+    qreal flickTo = ((cellSizeSmall + spacing) * 5);
     tableView->setContentX(flickTo);
     tableView->setContentY(flickTo);
 
@@ -537,7 +532,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     // Flick to row and column 20 (smallCellCount), since there the row and
     // column sizes increases with 100. Check that TableView then adjusts
     // contentWidth and contentHeight accordingly.
-    flickTo = ((cellSizeSmall + spacing) * smallCellCount) + margin - spacing;
+    flickTo = ((cellSizeSmall + spacing) * smallCellCount) - spacing;
     tableView->setContentX(flickTo);
     tableView->setContentY(flickTo);
 
@@ -549,7 +544,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     const qreal secondHalfOneScreenLength = largeSizeCellCountInView * cellSizeLarge;
     const qreal lengthAfterFlick = firstHalfLength + secondHalfOneScreenLength;
     const qreal averageCellSize = lengthAfterFlick / (smallCellCount + largeSizeCellCountInView);
-    const qreal expectedSizeHalf = (tableSize * averageCellSize) + spacingAndMargins;
+    const qreal expectedSizeHalf = (tableSize * averageCellSize) + accumulatedSpacing;
 
     QCOMPARE(tableView->contentWidth(), expectedSizeHalf);
     QCOMPARE(tableView->contentHeight(), expectedSizeHalf);
@@ -557,7 +552,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     // Flick to the end (row/column 100, and overshoot a bit), and
     // check that we then end up with the exact content width/height.
     const qreal secondHalfLength = largeCellCount * cellSizeLarge;
-    const qreal expectedFullSize = (firstHalfLength + secondHalfLength) + spacingAndMargins;
+    const qreal expectedFullSize = (firstHalfLength + secondHalfLength) + accumulatedSpacing;
     tableView->setContentX(expectedFullSize);
     tableView->setContentY(expectedFullSize);
 
@@ -713,6 +708,9 @@ void tst_QQuickTableView::checkLayoutOfEqualSizedDelegateItems()
     tableView->setModel(model);
     tableView->setRowSpacing(spacing.height());
     tableView->setColumnSpacing(spacing.width());
+
+    // Setting margins on Flickable should not affect the layout of the
+    // delegate items, since the margins is "transparent" to the TableView.
     tableView->setLeftMargin(margins.left());
     tableView->setTopMargin(margins.top());
     tableView->setRightMargin(margins.right());
@@ -729,8 +727,8 @@ void tst_QQuickTableView::checkLayoutOfEqualSizedDelegateItems()
         QCOMPARE(item->parentItem(), tableView->contentItem());
 
         const QPoint cell = getContextRowAndColumn(item);
-        qreal expectedX = margins.left() + (cell.x() * (expectedItemWidth + spacing.width()));
-        qreal expectedY = margins.top() + (cell.y() * (expectedItemHeight + spacing.height()));
+        qreal expectedX = cell.x() * (expectedItemWidth + spacing.width());
+        qreal expectedY = cell.y() * (expectedItemHeight + spacing.height());
         QCOMPARE(item->x(), expectedX);
         QCOMPARE(item->y(), expectedY);
         QCOMPARE(item->z(), 1);
@@ -782,83 +780,16 @@ void tst_QQuickTableView::checkFocusRemoved()
     }
 }
 
-void tst_QQuickTableView::checkTableMargins_data()
-{
-    QTest::addColumn<QVariant>("model");
-    QTest::addColumn<QSizeF>("spacing");
-    QTest::addColumn<QMarginsF>("margins");
-
-    QTest::newRow("QAIM single") << TestModelAsVariant(1, 1) << QSizeF(0, 0) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("QAIM single, 1,1, no margins") << TestModelAsVariant(1, 1) << QSizeF(1, 1) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("QAIM single, no spacing, 1111") << TestModelAsVariant(1, 1) << QSizeF(0, 0) << QMarginsF(1, 1, 1, 1);
-
-    QTest::newRow("QAIM 4x4") << TestModelAsVariant(4, 4) << QSizeF(0, 0) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("QAIM 4x4, 1,1, no margins") << TestModelAsVariant(4, 4) << QSizeF(1, 1) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("QAIM 4x4, no spacing, 1111") << TestModelAsVariant(1, 1) << QSizeF(0, 0) << QMarginsF(1, 1, 1, 1);
-
-    QTest::newRow("QAIM 1,1 0000") << TestModelAsVariant(20, 20) << QSizeF(1, 1) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("QAIM 1,1 5555") << TestModelAsVariant(20, 20) << QSizeF(1, 1) << QMarginsF(5, 5, 5, 5);
-    QTest::newRow("QAIM 0,0 3333") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(3, 3, 3, 3);
-    QTest::newRow("QAIM 2,2 1234") << TestModelAsVariant(20, 20) << QSizeF(2, 2) << QMarginsF(1, 2, 3, 4);
-    QTest::newRow("QAIM 0,0 3210") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(3, 2, 1, 0);
-
-    QTest::newRow("QAIM 0,0 negative left margin") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(-10, 0, 0, 0);
-    QTest::newRow("QAIM 0,0 negative top margin") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(0, -10, 0, 0);
-    QTest::newRow("QAIM 0,0 negative right margin") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(0, 0, -10, 0);
-    QTest::newRow("QAIM 0,0 negative bottom margin") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(0, 0, 0, -10);
-    QTest::newRow("QAIM 0,0 all margins negative") << TestModelAsVariant(20, 20) << QSizeF(0, 0) << QMarginsF(-10, -10, -10, -10);
-}
-
-void tst_QQuickTableView::checkTableMargins()
-{
-    // Check that the space between the content view and
-    // the items matches the margins we set on the tableview.
-    QFETCH(QVariant, model);
-    QFETCH(QSizeF, spacing);
-    QFETCH(QMarginsF, margins);
-    LOAD_TABLEVIEW("plaintableview.qml");
-
-    tableView->setModel(model);
-    tableView->setRowSpacing(spacing.height());
-    tableView->setColumnSpacing(spacing.width());
-    tableView->setLeftMargin(margins.left());
-    tableView->setTopMargin(margins.top());
-    tableView->setRightMargin(margins.right());
-    tableView->setBottomMargin(margins.bottom());
-
-    WAIT_UNTIL_POLISHED;
-
-    // Check left-, and top margins
-    auto const topLeftItem = tableViewPrivate->loadedTableItem(QPoint(0, 0))->item;
-    qreal leftSpace = topLeftItem->x();
-    qreal topSpace = topLeftItem->y();
-    QCOMPARE(leftSpace, margins.left());
-    QCOMPARE(topSpace, margins.top());
-
-    // Flick the table to the end...
-    tableView->setContentX(tableView->contentWidth() - tableView->width());
-    tableView->setContentY(tableView->contentHeight() - tableView->height());
-    const QPoint bottomRightCell = tableViewPrivate->loadedTable.bottomRight();
-    auto const bottomRightItem = tableViewPrivate->loadedTableItem(bottomRightCell)->item;
-
-    // ...and check the right-, and bottom margins
-    qreal rightSpace = tableView->contentWidth() - (bottomRightItem->x() + bottomRightItem->width());
-    qreal bottomSpace = tableView->contentHeight() - (bottomRightItem->y() + bottomRightItem->height());
-    QCOMPARE(rightSpace, margins.right());
-    QCOMPARE(bottomSpace, margins.bottom());
-}
-
 void tst_QQuickTableView::fillTableViewButNothingMore_data()
 {
     QTest::addColumn<QSizeF>("spacing");
-    QTest::addColumn<QMarginsF>("margins");
 
-    QTest::newRow("0 0,0 0") << QSizeF(0, 0) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("0 10,10 0") << QSizeF(10, 10) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("100 10,10 0") << QSizeF(10, 10) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("0 0,0 100") << QSizeF(0, 0) << QMarginsF(0, 0, 0, 0);
-    QTest::newRow("0 10,10 100") << QSizeF(10, 10) << QMarginsF(100, 100, 100, 100);
-    QTest::newRow("100 10,10 100") << QSizeF(10, 10) << QMarginsF(100, 100, 100, 100);
+    QTest::newRow("0 0,0 0") << QSizeF(0, 0);
+    QTest::newRow("0 10,10 0") << QSizeF(10, 10);
+    QTest::newRow("100 10,10 0") << QSizeF(10, 10);
+    QTest::newRow("0 0,0 100") << QSizeF(0, 0);
+    QTest::newRow("0 10,10 100") << QSizeF(10, 10);
+    QTest::newRow("100 10,10 100") << QSizeF(10, 10);
 }
 
 void tst_QQuickTableView::fillTableViewButNothingMore()
@@ -866,7 +797,6 @@ void tst_QQuickTableView::fillTableViewButNothingMore()
     // Check that we end up filling the whole visible part of
     // the tableview with cells, but nothing more.
     QFETCH(QSizeF, spacing);
-    QFETCH(QMarginsF, margins);
     LOAD_TABLEVIEW("plaintableview.qml");
 
     const int rows = 100;
@@ -876,20 +806,12 @@ void tst_QQuickTableView::fillTableViewButNothingMore()
     tableView->setModel(model);
     tableView->setRowSpacing(spacing.height());
     tableView->setColumnSpacing(spacing.width());
-    tableView->setLeftMargin(margins.left());
-    tableView->setTopMargin(margins.top());
-    tableView->setRightMargin(margins.right());
-    tableView->setBottomMargin(margins.bottom());
 
     WAIT_UNTIL_POLISHED;
 
     auto const topLeftFxItem = tableViewPrivate->loadedTableItem(QPoint(0, 0));
     auto const topLeftItem = topLeftFxItem->item;
 
-    // Check that the top-left item are at the corner of the view
-    QCOMPARE(topLeftItem->x(), margins.left());
-    QCOMPARE(topLeftItem->y(), margins.top());
-
     auto const bottomRightFxItem = tableViewPrivate->loadedTableItem(tableViewPrivate->loadedTable.bottomRight());
     auto const bottomRightItem = bottomRightFxItem->item;
     const QPoint bottomRightCell = getContextRowAndColumn(bottomRightItem.data());
@@ -900,8 +822,7 @@ void tst_QQuickTableView::fillTableViewButNothingMore()
 
     // Check that the actual number of columns matches what we expect
     qreal cellWidth = bottomRightItem->width() + spacing.width();
-    qreal availableWidth = tableView->width() - margins.left();
-    int expectedColumns = qCeil(availableWidth / cellWidth);
+    int expectedColumns = qCeil(tableView->width() / cellWidth);
     int actualColumns = bottomRightCell.x() + 1;
     QCOMPARE(actualColumns, expectedColumns);
 
@@ -911,8 +832,7 @@ void tst_QQuickTableView::fillTableViewButNothingMore()
 
     // Check that the actual number of rows matches what we expect
     qreal cellHeight = bottomRightItem->height() + spacing.height();
-    qreal availableHeight = tableView->height() - margins.top();
-    int expectedRows = qCeil(availableHeight / cellHeight);
+    int expectedRows = qCeil(tableView->height() / cellHeight);
     int actualRows = bottomRightCell.y() + 1;
     QCOMPARE(actualRows, expectedRows);
 }