From 80b5612708a115613c534644053eb0d8b2cda108 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Wed, 21 Mar 2012 12:55:36 +1000 Subject: [PATCH] Fix margins for right-to-left mode The behaviour for considering left and right margins was inconsistent in views with a right to left layout; these values were reversed for extent calculations but not for general positioning. With this change the left and right margins are never reversed in a right-to-left layout, so minXExtent and maxXExtent calculations always use startMargin and endMargin respectively, regardless of layout direction. Also fixes calculation of endOffset in trackedPositionChanged() when in horizontal orientation. Change-Id: Ie00e3d4c2bd38d8fe6ac0213702206b88bfa895e Reviewed-by: Martin Jones --- src/quick/items/qquickitemview.cpp | 12 +++---- .../qquickgridview/tst_qquickgridview.cpp | 23 +++++++------- .../quick/qquicklistview/data/margins2.qml | 4 +-- .../qquicklistview/tst_qquicklistview.cpp | 31 +++++++++++++++++-- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 50a3216bf0..0d95500860 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -1101,9 +1101,9 @@ void QQuickItemView::trackedPositionChanged() if (d->layoutOrientation() == Qt::Vertical) endOffset += d->vData.endMargin; else if (d->isContentFlowReversed()) - endOffset += d->hData.endMargin; - else endOffset += d->hData.startMargin; + else + endOffset += d->hData.endMargin; trackedPos += endOffset; trackedEndPos += endOffset; toItemPos += endOffset; @@ -1204,12 +1204,11 @@ qreal QQuickItemView::minXExtent() const return QQuickFlickable::minXExtent(); if (d->hData.minExtentDirty) { - d->minExtent = -d->startPosition(); + d->minExtent = -d->startPosition() + d->hData.startMargin; qreal highlightStart; qreal highlightEnd; qreal endPositionFirstItem = 0; if (d->isContentFlowReversed()) { - d->minExtent += d->hData.endMargin; if (d->model && d->model->count()) endPositionFirstItem = d->positionAt(d->model->count()-1); else if (d->header) @@ -1222,7 +1221,6 @@ qreal QQuickItemView::minXExtent() const if (d->minExtent < maxX) d->minExtent = maxX; } else { - d->minExtent += d->hData.startMargin; endPositionFirstItem = d->endPositionAt(0); highlightStart = d->highlightRangeStart; highlightEnd = d->highlightRangeEnd; @@ -1279,7 +1277,7 @@ qreal QQuickItemView::maxXExtent() const if (d->isContentFlowReversed()) { if (d->header) d->maxExtent -= d->headerSize(); - d->maxExtent -= d->hData.startMargin; + d->maxExtent -= d->hData.endMargin; } else { if (d->footer) d->maxExtent -= d->footerSize(); @@ -1314,7 +1312,7 @@ qreal QQuickItemView::xOrigin() const { Q_D(const QQuickItemView); if (d->isContentFlowReversed()) - return -maxXExtent() + d->size() - d->hData.startMargin; + return -maxXExtent() + d->size() - d->hData.endMargin; else return -minXExtent() + d->hData.startMargin; } diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 88ed94d8bc..c7b5ca6b40 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -3691,31 +3691,32 @@ void tst_QQuickGridView::margins() QQuickItem *contentItem = gridview->contentItem(); QTRY_VERIFY(contentItem != 0); - QCOMPARE(gridview->contentX(), -240+30.); - QCOMPARE(gridview->xOrigin(), 0.); + QTRY_COMPARE(gridview->contentX(), -240+50.); + QTRY_COMPARE(gridview->xOrigin(), 0.); // check end bound gridview->positionViewAtEnd(); qreal pos = gridview->contentX(); gridview->setContentX(pos - 80); gridview->returnToBounds(); - QTRY_COMPARE(gridview->contentX(), pos - 50); + QTRY_COMPARE(gridview->contentX(), pos - 30); // remove item before visible and check that left margin is maintained // and xOrigin is updated gridview->setContentX(-400); + QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); model.removeItems(0, 4); - QTest::qWait(100); + QTRY_COMPARE(model.count(), gridview->count()); gridview->setContentX(-240+50); gridview->returnToBounds(); QCOMPARE(gridview->xOrigin(), -100.); - QTRY_COMPARE(gridview->contentX(), -240-70.); + QTRY_COMPARE(gridview->contentX(), -240-50.); - // reduce left margin (i.e. right side due to RTL) + // reduce right margin pos = gridview->contentX(); - gridview->setLeftMargin(20); + gridview->setRightMargin(40); QCOMPARE(gridview->xOrigin(), -100.); - QTRY_COMPARE(gridview->contentX(), -240-80.); + QTRY_COMPARE(gridview->contentX(), -240-100 + 40.); // check end bound gridview->positionViewAtEnd(); @@ -3723,11 +3724,11 @@ void tst_QQuickGridView::margins() pos = gridview->contentX(); gridview->setContentX(pos - 80); gridview->returnToBounds(); - QTRY_COMPARE(gridview->contentX(), pos - 50); + QTRY_COMPARE(gridview->contentX(), pos - 30); - // reduce right margin (i.e. left side due to RTL) + // reduce left margin pos = gridview->contentX(); - gridview->setRightMargin(40); + gridview->setLeftMargin(20); QCOMPARE(gridview->xOrigin(), 0.); QTRY_COMPARE(gridview->contentX(), pos+10); diff --git a/tests/auto/quick/qquicklistview/data/margins2.qml b/tests/auto/quick/qquicklistview/data/margins2.qml index e11c803c4b..4b1f2546bf 100644 --- a/tests/auto/quick/qquicklistview/data/margins2.qml +++ b/tests/auto/quick/qquicklistview/data/margins2.qml @@ -9,9 +9,9 @@ Item { } ListView { objectName: "listview" - topMargin: 20 + topMargin: 40 bottomMargin: 20 - leftMargin: 20 + leftMargin: 40 rightMargin: 20 anchors.fill: parent diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index 202f5164af..461a6b6750 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -4290,6 +4290,13 @@ void tst_QQuickListView::marginsResize() QFETCH(qreal, start); QFETCH(qreal, end); + QPoint flickStart(20, 20); + QPoint flickEnd(20, 20); + if (orientation == QQuickListView::Vertical) + flickStart.ry() += 180; + else + flickStart.rx() += (layoutDirection == Qt::LeftToRight) ? 180 : -180; + QQuickView *canvas = getView(); canvas->setSource(testFileUrl("margins2.qml")); @@ -4316,6 +4323,14 @@ void tst_QQuickListView::marginsResize() else QTRY_COMPARE(listview->contentX(), end); + // flick past the end and check content pos still settles on correct extents + flick(canvas, flickStart, flickEnd, 180); + QTRY_VERIFY(listview->isMoving() == false); + if (orientation == QQuickListView::Vertical) + QTRY_COMPARE(listview->contentY(), end); + else + QTRY_COMPARE(listview->contentX(), end); + // back to top - top margin should be visible. listview->setCurrentIndex(0); if (orientation == QQuickListView::Vertical) @@ -4323,6 +4338,14 @@ void tst_QQuickListView::marginsResize() else QTRY_COMPARE(listview->contentX(), start); + // flick past the beginning and check content pos still settles on correct extents + flick(canvas, flickEnd, flickStart, 180); + QTRY_VERIFY(listview->isMoving() == false); + if (orientation == QQuickListView::Vertical) + QTRY_COMPARE(listview->contentY(), start); + else + QTRY_COMPARE(listview->contentX(), start); + releaseView(canvas); } @@ -4333,9 +4356,11 @@ void tst_QQuickListView::marginsResize_data() QTest::addColumn("start"); QTest::addColumn("end"); - QTest::newRow("vertical") << QQuickListView::Vertical << Qt::LeftToRight << -20.0 << 1020.0; - QTest::newRow("horizontal") << QQuickListView::Horizontal << Qt::LeftToRight << -20.0 << 1020.0; - QTest::newRow("horizontal, rtl") << QQuickListView::Horizontal << Qt::RightToLeft << -180.0 << -1220.0; + // in Right to Left mode, leftMargin still means leftMargin - it doesn't reverse to mean rightMargin + + QTest::newRow("vertical") << QQuickListView::Vertical << Qt::LeftToRight << -40.0 << 1020.0; + QTest::newRow("horizontal") << QQuickListView::Horizontal << Qt::LeftToRight << -40.0 << 1020.0; + QTest::newRow("horizontal, rtl") << QQuickListView::Horizontal << Qt::RightToLeft << -180.0 << -1240.0; } void tst_QQuickListView::snapToItem_data()