From da0441aa30e1e10c1e0ac9a8cd305c1b6e63c111 Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Thu, 28 Jul 2011 15:06:02 +1000 Subject: [PATCH] ListView doesn't show new 1st item currentItem is removed. If the new 1st item becomes the currentItem after removal it was positioned incorrectly because the currentItem is positioned based on visibleIndex, which was updated after the currentItem was updated. Move visibleIndex update before currentItem update. Change-Id: Iaf92a41eefe7bce093e3000d17f5496dba144bcd Fixes: QTBUG-20575 Reviewed-on: http://codereview.qt.nokia.com/2316 Reviewed-by: Qt Sanity Bot Reviewed-by: Bea Lam --- src/declarative/items/qsggridview.cpp | 18 +++++++++--------- src/declarative/items/qsglistview.cpp | 20 ++++++++++---------- .../graphicsitems/qdeclarativegridview.cpp | 18 +++++++++--------- .../graphicsitems/qdeclarativelistview.cpp | 20 ++++++++++---------- .../declarative/qsglistview/tst_qsglistview.cpp | 9 ++++++++- .../tst_qdeclarativelistview.cpp | 9 ++++++++- 6 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/declarative/items/qsggridview.cpp b/src/declarative/items/qsggridview.cpp index 214481a..7b0e4f3 100644 --- a/src/declarative/items/qsggridview.cpp +++ b/src/declarative/items/qsggridview.cpp @@ -1445,6 +1445,15 @@ void QSGGridView::itemsRemoved(int modelIndex, int count) } } + // update visibleIndex + d->visibleIndex = 0; + for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { + if ((*it)->index != -1) { + d->visibleIndex = (*it)->index; + break; + } + } + // fix current if (d->currentIndex >= modelIndex + count) { d->currentIndex -= count; @@ -1462,15 +1471,6 @@ void QSGGridView::itemsRemoved(int modelIndex, int count) emit currentIndexChanged(); } - // update visibleIndex - d->visibleIndex = 0; - for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { - if ((*it)->index != -1) { - d->visibleIndex = (*it)->index; - break; - } - } - if (removedVisible && d->visibleItems.isEmpty()) { d->timeline.clear(); if (d->itemCount == 0) { diff --git a/src/declarative/items/qsglistview.cpp b/src/declarative/items/qsglistview.cpp index 37b027c..641dd35 100644 --- a/src/declarative/items/qsglistview.cpp +++ b/src/declarative/items/qsglistview.cpp @@ -1751,6 +1751,16 @@ void QSGListView::itemsRemoved(int modelIndex, int count) if (firstVisible && d->visibleItems.first() != firstVisible) static_cast(d->visibleItems.first())->setPosition(d->visibleItems.first()->position() + preRemovedSize); + // update visibleIndex + bool haveVisibleIndex = false; + for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { + if ((*it)->index != -1) { + d->visibleIndex = (*it)->index; + haveVisibleIndex = true; + break; + } + } + // fix current if (d->currentIndex >= modelIndex + count) { d->currentIndex -= count; @@ -1771,16 +1781,6 @@ void QSGListView::itemsRemoved(int modelIndex, int count) emit currentIndexChanged(); } - // update visibleIndex - bool haveVisibleIndex = false; - for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { - if ((*it)->index != -1) { - d->visibleIndex = (*it)->index; - haveVisibleIndex = true; - break; - } - } - if (!haveVisibleIndex) { d->timeline.clear(); if (removedVisible && d->itemCount == 0) { diff --git a/src/qtquick1/graphicsitems/qdeclarativegridview.cpp b/src/qtquick1/graphicsitems/qdeclarativegridview.cpp index f81256c..63b907b 100644 --- a/src/qtquick1/graphicsitems/qdeclarativegridview.cpp +++ b/src/qtquick1/graphicsitems/qdeclarativegridview.cpp @@ -2919,6 +2919,15 @@ void QDeclarative1GridView::itemsRemoved(int modelIndex, int count) } } + // update visibleIndex + d->visibleIndex = 0; + for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { + if ((*it)->index != -1) { + d->visibleIndex = (*it)->index; + break; + } + } + // fix current if (d->currentIndex >= modelIndex + count) { d->currentIndex -= count; @@ -2936,15 +2945,6 @@ void QDeclarative1GridView::itemsRemoved(int modelIndex, int count) emit currentIndexChanged(); } - // update visibleIndex - d->visibleIndex = 0; - for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { - if ((*it)->index != -1) { - d->visibleIndex = (*it)->index; - break; - } - } - if (removedVisible && d->visibleItems.isEmpty()) { d->timeline.clear(); if (d->itemCount == 0) { diff --git a/src/qtquick1/graphicsitems/qdeclarativelistview.cpp b/src/qtquick1/graphicsitems/qdeclarativelistview.cpp index 4c01514..c58543c 100644 --- a/src/qtquick1/graphicsitems/qdeclarativelistview.cpp +++ b/src/qtquick1/graphicsitems/qdeclarativelistview.cpp @@ -3383,6 +3383,16 @@ void QDeclarative1ListView::itemsRemoved(int modelIndex, int count) if (firstVisible && d->visibleItems.first() != firstVisible) d->visibleItems.first()->setPosition(d->visibleItems.first()->position() + preRemovedSize); + // update visibleIndex + bool haveVisibleIndex = false; + for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { + if ((*it)->index != -1) { + d->visibleIndex = (*it)->index; + haveVisibleIndex = true; + break; + } + } + // fix current if (d->currentIndex >= modelIndex + count) { d->currentIndex -= count; @@ -3401,16 +3411,6 @@ void QDeclarative1ListView::itemsRemoved(int modelIndex, int count) emit currentIndexChanged(); } - // update visibleIndex - bool haveVisibleIndex = false; - for (it = d->visibleItems.begin(); it != d->visibleItems.end(); ++it) { - if ((*it)->index != -1) { - d->visibleIndex = (*it)->index; - haveVisibleIndex = true; - break; - } - } - if (!haveVisibleIndex) { d->timeline.clear(); if (removedVisible && d->itemCount == 0) { diff --git a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp index c7a68e5..43efea2 100644 --- a/tests/auto/declarative/qsglistview/tst_qsglistview.cpp +++ b/tests/auto/declarative/qsglistview/tst_qsglistview.cpp @@ -788,9 +788,16 @@ void tst_QSGListView::removed(bool animated) QCOMPARE(name->text(), QString("New")); // Add some more items so that we don't run out - for (int i = 50; i < 100; i++) + model.clear(); + for (int i = 0; i < 50; i++) model.addItem("Item" + QString::number(i), ""); + // QTBUG-QTBUG-20575 + listview->setCurrentIndex(0); + listview->setContentY(30); + model.removeItem(0); + QTRY_VERIFY(name = findItem(contentItem, "textName", 0)); + // QTBUG-19198 move to end and remove all visible items one at a time. listview->positionViewAtEnd(); for (int i = 0; i < 18; ++i) diff --git a/tests/auto/qtquick1/qdeclarativelistview/tst_qdeclarativelistview.cpp b/tests/auto/qtquick1/qdeclarativelistview/tst_qdeclarativelistview.cpp index 5bfa08e..d2519b0 100644 --- a/tests/auto/qtquick1/qdeclarativelistview/tst_qdeclarativelistview.cpp +++ b/tests/auto/qtquick1/qdeclarativelistview/tst_qdeclarativelistview.cpp @@ -725,9 +725,16 @@ void tst_QDeclarative1ListView::removed(bool animated) QCOMPARE(name->text(), QString("New")); // Add some more items so that we don't run out - for (int i = 50; i < 100; i++) + model.clear(); + for (int i = 0; i < 50; i++) model.addItem("Item" + QString::number(i), ""); + // QTBUG-QTBUG-20575 + listview->setCurrentIndex(0); + listview->setContentY(30); + model.removeItem(0); + QTRY_VERIFY(name = findItem(contentItem, "textName", 0)); + // QTBUG-19198 move to end and remove all visible items one at a time. listview->positionViewAtEnd(); for (int i = 0; i < 18; ++i) -- 1.7.2.5