From 54da515e5e5f65e7ae253280038aad912780de12 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Fri, 23 Sep 2011 11:40:25 +1000 Subject: [PATCH] Don't move content forwards when items are moved down Previously, if items moved down within a view, the content position would effectively drop down. E.g. for a (0,5,3) move that moved 3 items from 0 to 5, the content y would move to the position of index 3, since it became the new first item. However, this makes it difficult to move transitions for move() operations in these cases since these items do not move (since the content position moves instead). With this fix, the content position does not move, and items will always move if they are moved. Note this behaviour was previously implemented for backwards movements, e.g. a (5,0,3) move but was not enabled for a forwards (0,5,3) move. Change-Id: I1c5a19e3c36347a4aa0cf6e31c975967a7eeada9 Reviewed-on: http://codereview.qt-project.org/5576 Reviewed-by: Qt Sanity Bot Reviewed-by: Martin Jones --- src/declarative/items/qsggridview.cpp | 2 ++ src/declarative/items/qsgitemview.cpp | 15 +++++++++++++-- src/declarative/items/qsglistview.cpp | 2 ++ tests/auto/declarative/qsggridview/tst_qsggridview.cpp | 9 +++++++-- tests/auto/declarative/qsglistview/tst_qsglistview.cpp | 11 ++++++++--- 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp index dfcd1b2..099d62e 100644 --- a/src/declarative/items/qsggridview.cpp +++ b/src/declarative/items/qsggridview.cpp @@ -574,6 +574,8 @@ void QSGGridViewPrivate::repositionPackageItemAt(QSGItem *item, int index) void QSGGridViewPrivate::resetItemPosition(FxViewItem *item, FxViewItem *toItem) { + if (item == toItem) + return; FxGridItemSG *toGridItem = static_cast(toItem); static_cast(item)->setPosition(toGridItem->colPos(), toGridItem->rowPos()); } diff --git a/src/declarative/items/qsgitemview.cpp b/src/declarative/items/qsgitemview.cpp index 63040c1..666b471 100644 --- a/src/declarative/items/qsgitemview.cpp +++ b/src/declarative/items/qsgitemview.cpp @@ -1489,16 +1489,27 @@ bool QSGItemViewPrivate::applyModelChanges() for (int i=0; iattached->emitAdd(); - // if first visible item is moving but another item is moving up to replace it, - // do this positioning now to avoid shifting all content forwards + // if the first visible item has moved, ensure another one takes its place + // so that we avoid shifting all content forwards + // (don't use items from removedBeforeFirstVisible - if an item is removed from + // before the first visible, the first visible should not move upwards) if (firstVisible && firstItemIndex >= 0) { + bool found = false; for (int i=0; iindex == firstItemIndex) { + // an item has moved backwards up to the first visible's position resetItemPosition(movedBackwards[i], firstVisible); movedBackwards.removeAt(i); + found = true; break; } } + if (!found) { + // first visible item has moved forward, another item takes its place + FxViewItem *item = visibleItem(firstItemIndex); + if (item) + resetItemPosition(item, firstVisible); + } } // Ensure we don't cause an ugly list scroll diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp index 4aaf131..7f9ce3a 100644 --- a/src/declarative/items/qsglistview.cpp +++ b/src/declarative/items/qsglistview.cpp @@ -716,6 +716,8 @@ void QSGListViewPrivate::repositionPackageItemAt(QSGItem *item, int index) void QSGListViewPrivate::resetItemPosition(FxViewItem *item, FxViewItem *toItem) { + if (item == toItem) + return; static_cast(item)->setPosition(toItem->position()); } diff --git a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp index 6ec5378..eab291f 100644 --- a/tests/auto/declarative/qsggridview/tst_qsggridview.cpp +++ b/tests/auto/declarative/qsggridview/tst_qsggridview.cpp @@ -737,7 +737,12 @@ void tst_QSGGridView::moved_data() QTest::newRow("move multiple forwards, within visible items") << 0.0 << 0 << 5 << 3 - << 60.0; // moved 3 items (i.e. 1 row) down + << 0.0; + + QTest::newRow("move multiple forwards, before visible items") + << 120.0 // show 6-23 + << 3 << 4 << 3 // 3, 4, 5 move to after 6 + << 60.0; // row of 3,4,5 has moved down QTest::newRow("move multiple forwards, from non-visible -> visible") << 120.0 // show 6-23 @@ -757,7 +762,7 @@ void tst_QSGGridView::moved_data() QTest::newRow("move multiple forwards, from visible -> non-visible (move first item)") << 0.0 << 0 << 16 << 3 - << 60.0; + << 0.0; QTest::newRow("move multiple backwards, within visible items") diff --git a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp index 9642b8f..c1be773 100644 --- a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp +++ b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp @@ -981,7 +981,7 @@ void tst_QSGListView::moved_data() QTest::newRow("move 1 forwards, from visible -> non-visible (move first item)") << 0.0 << 0 << 16 << 1 - << 20.0; + << 0.0; QTest::newRow("move 1 backwards, within visible items") @@ -1018,7 +1018,12 @@ void tst_QSGListView::moved_data() QTest::newRow("move multiple forwards, within visible items") << 0.0 << 0 << 5 << 3 - << 20.0 * 3; + << 0.0; + + QTest::newRow("move multiple forwards, before visible items") + << 140.0 // show 7-22 + << 4 << 5 << 3 // 4,5,6 move to below 7 + << 20.0 * 3; // 4,5,6 moved down QTest::newRow("move multiple forwards, from non-visible -> visible") << 80.0 // show 4-19 @@ -1038,7 +1043,7 @@ void tst_QSGListView::moved_data() QTest::newRow("move multiple forwards, from visible -> non-visible (move first item)") << 0.0 << 0 << 16 << 3 - << 20.0 * 3; + << 0.0; QTest::newRow("move multiple backwards, within visible items") -- 2.7.4