Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 <martin.jones@nokia.com>
  • Loading branch information
Bea Lam authored and Qt by Nokia committed Mar 21, 2012
1 parent dd952a2 commit 80b5612
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 23 deletions.
12 changes: 5 additions & 7 deletions src/quick/items/qquickitemview.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
23 changes: 12 additions & 11 deletions tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
Expand Up @@ -3691,43 +3691,44 @@ 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();
QCOMPARE(gridview->xOrigin(), 0.); // positionViewAtEnd() resets origin
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);

Expand Down
4 changes: 2 additions & 2 deletions tests/auto/quick/qquicklistview/data/margins2.qml
Expand Up @@ -9,9 +9,9 @@ Item {
}
ListView {
objectName: "listview"
topMargin: 20
topMargin: 40
bottomMargin: 20
leftMargin: 20
leftMargin: 40
rightMargin: 20
anchors.fill: parent

Expand Down
31 changes: 28 additions & 3 deletions tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
Expand Up @@ -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"));
Expand All @@ -4316,13 +4323,29 @@ 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)
QTRY_COMPARE(listview->contentY(), start);
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);
}

Expand All @@ -4333,9 +4356,11 @@ void tst_QQuickListView::marginsResize_data()
QTest::addColumn<qreal>("start");
QTest::addColumn<qreal>("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()
Expand Down

0 comments on commit 80b5612

Please sign in to comment.