From: Frank Reininghaus Date: Thu, 11 Aug 2011 10:23:21 +0000 (+0200) Subject: Emit KitemListSelectionManager's selectionChanged signal correctly X-Git-Url: https://cloud.milkyroute.net/gitweb/dolphin.git/commitdiff_plain/01ff60edeea76f77a5c4684d1f674efb81418faa?ds=inline Emit KitemListSelectionManager's selectionChanged signal correctly This commit makes sure that the signal is emitted with the correct current and previous selection after a selection change, and that the signal is emitted exactly once in KItemListSelectionManager::itemsInserted and KItemListSelectionManager::itemsRemoved. --- diff --git a/src/kitemviews/kitemlistselectionmanager.cpp b/src/kitemviews/kitemlistselectionmanager.cpp index 18fa2f660..cfe847b69 100644 --- a/src/kitemviews/kitemlistselectionmanager.cpp +++ b/src/kitemviews/kitemlistselectionmanager.cpp @@ -43,6 +43,8 @@ KItemListSelectionManager::~KItemListSelectionManager() void KItemListSelectionManager::setCurrentItem(int current) { const int previous = m_currentItem; + const QSet previousSelection = selectedItems(); + if (m_model && current >= 0 && current < m_model->count()) { m_currentItem = current; } else { @@ -51,6 +53,13 @@ void KItemListSelectionManager::setCurrentItem(int current) if (m_currentItem != previous) { emit currentChanged(m_currentItem, previous); + + if (m_isAnchoredSelectionActive) { + const QSet selection = selectedItems(); + if (selection != previousSelection) { + emit selectionChanged(selection, previousSelection); + } + } } } @@ -95,7 +104,7 @@ void KItemListSelectionManager::setSelected(int index, int count, SelectionMode return; } - const QSet previous = m_selectedItems; + const QSet previous = selectedItems(); count = qMin(count, m_model->count() - index); @@ -128,23 +137,19 @@ void KItemListSelectionManager::setSelected(int index, int count, SelectionMode break; } - if (m_selectedItems != previous) { - emit selectionChanged(m_selectedItems, previous); + const QSet selection = selectedItems(); + if (selection != previous) { + emit selectionChanged(selection, previous); } } void KItemListSelectionManager::clearSelection() { - if (!m_selectedItems.isEmpty()) { - const QSet previous = m_selectedItems; + const QSet previous = selectedItems(); + if (!previous.isEmpty()) { m_selectedItems.clear(); m_isAnchoredSelectionActive = false; - emit selectionChanged(m_selectedItems, previous); - } - else if (m_isAnchoredSelectionActive) { - m_isAnchoredSelectionActive = false; - // TODO: the 'previous' parameter of the signal has to be set correctly, but do we actually need it? - emit selectionChanged(m_selectedItems, m_selectedItems); + emit selectionChanged(QSet(), previous); } } @@ -212,10 +217,14 @@ void KItemListSelectionManager::setModel(KItemModelBase* model) void KItemListSelectionManager::itemsInserted(const KItemRangeList& itemRanges) { + // Store the current selection (needed in the selectionChanged() signal) + const QSet previousSelection = selectedItems(); + // Update the current item if (m_currentItem < 0) { setCurrentItem(0); } else { + const int previousCurrent = m_currentItem; int inc = 0; foreach (const KItemRange& itemRange, itemRanges) { if (m_currentItem < itemRange.index) { @@ -223,13 +232,17 @@ void KItemListSelectionManager::itemsInserted(const KItemRangeList& itemRanges) } inc += itemRange.count; } - setCurrentItem(m_currentItem + inc); + // Calling setCurrentItem would trigger the selectionChanged signal, but we want to + // emit it only once in this function -> change the current item manually and emit currentChanged + m_currentItem += inc; + emit currentChanged(m_currentItem, previousCurrent); } // Update the anchor item if (m_anchorItem < 0) { setAnchorItem(0); } else { + const int previousAnchor = m_anchorItem; int inc = 0; foreach (const KItemRange& itemRange, itemRanges) { if (m_anchorItem < itemRange.index) { @@ -237,16 +250,15 @@ void KItemListSelectionManager::itemsInserted(const KItemRangeList& itemRanges) } inc += itemRange.count; } - setAnchorItem(m_anchorItem + inc); + m_anchorItem += inc; + emit anchorChanged(m_anchorItem, previousAnchor); } // Update the selections if (!m_selectedItems.isEmpty()) { const QSet previous = m_selectedItems; - - QSet current; - current.reserve(m_selectedItems.count()); - QSetIterator it(m_selectedItems); + m_selectedItems.clear(); + QSetIterator it(previous); while (it.hasNext()) { const int index = it.next(); int inc = 0; @@ -256,20 +268,24 @@ void KItemListSelectionManager::itemsInserted(const KItemRangeList& itemRanges) } inc += itemRange.count; } - current.insert(index + inc); + m_selectedItems.insert(index + inc); } + } - if (current != previous) { - m_selectedItems = current; - emit selectionChanged(current, previous); - } + const QSet selection = selectedItems(); + if (selection != previousSelection) { + emit selectionChanged(selection, previousSelection); } } void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges) { + // Store the current selection (needed in the selectionChanged() signal) + const QSet previousSelection = selectedItems(); + // Update the current item if (m_currentItem >= 0) { + const int previousCurrent = m_currentItem; int currentItem = m_currentItem; foreach (const KItemRange& itemRange, itemRanges) { if (currentItem < itemRange.index) { @@ -281,11 +297,15 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges) currentItem = m_model->count() - 1; } } - setCurrentItem(currentItem); + // Calling setCurrentItem would trigger the selectionChanged signal, but we want to + // emit it only once in this function -> change the current item manually and emit currentChanged + m_currentItem = currentItem; + emit currentChanged(m_currentItem, previousCurrent); } // Update the anchor item if (m_anchorItem >= 0) { + const int previousAnchor = m_anchorItem; int anchorItem = m_anchorItem; foreach (const KItemRange& itemRange, itemRanges) { if (anchorItem < itemRange.index) { @@ -297,16 +317,15 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges) anchorItem = m_model->count() - 1; } } - setAnchorItem(anchorItem); + m_anchorItem = anchorItem; + emit anchorChanged(m_anchorItem, previousAnchor); } // Update the selections if (!m_selectedItems.isEmpty()) { const QSet previous = m_selectedItems; - - QSet current; - current.reserve(m_selectedItems.count()); - QSetIterator it(m_selectedItems); + m_selectedItems.clear(); + QSetIterator it(previous); while (it.hasNext()) { int index = it.next(); int dec = 0; @@ -326,14 +345,14 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges) } index -= dec; if (index >= 0) { - current.insert(index); + m_selectedItems.insert(index); } } + } - if (current != previous) { - m_selectedItems = current; - emit selectionChanged(current, previous); - } + const QSet selection = selectedItems(); + if (selection != previousSelection) { + emit selectionChanged(selection, previousSelection); } } diff --git a/src/tests/kitemlistselectionmanagertest.cpp b/src/tests/kitemlistselectionmanagertest.cpp index 4c45fd1b8..a79b9e6f3 100644 --- a/src/tests/kitemlistselectionmanagertest.cpp +++ b/src/tests/kitemlistselectionmanagertest.cpp @@ -212,6 +212,7 @@ void KItemListSelectionManagerTest::testItemsInserted() // Select items 10 to 12 m_selectionManager->setSelected(10, 3); QSet selectedItems = m_selectionManager->selectedItems(); + QCOMPARE(selectedItems.count(), 3); QVERIFY(selectedItems.contains(10)); QVERIFY(selectedItems.contains(11)); QVERIFY(selectedItems.contains(12)); @@ -219,6 +220,7 @@ void KItemListSelectionManagerTest::testItemsInserted() // Insert items 0 to 4 -> selection must be 15 to 17 m_selectionManager->itemsInserted(KItemRangeList() << KItemRange(0, 5)); selectedItems = m_selectionManager->selectedItems(); + QCOMPARE(selectedItems.count(), 3); QVERIFY(selectedItems.contains(15)); QVERIFY(selectedItems.contains(16)); QVERIFY(selectedItems.contains(17)); @@ -229,6 +231,7 @@ void KItemListSelectionManagerTest::testItemsInserted() KItemRange(16, 1) << KItemRange(17, 1)); selectedItems = m_selectionManager->selectedItems(); + QCOMPARE(selectedItems.count(), 3); QVERIFY(selectedItems.contains(16)); QVERIFY(selectedItems.contains(18)); QVERIFY(selectedItems.contains(20)); @@ -239,6 +242,7 @@ void KItemListSelectionManagerTest::testItemsRemoved() // Select items 10 to 15 m_selectionManager->setSelected(10, 6); QSet selectedItems = m_selectionManager->selectedItems(); + QCOMPARE(selectedItems.count(), 6); for (int i = 10; i <= 15; ++i) { QVERIFY(selectedItems.contains(i)); } @@ -246,6 +250,7 @@ void KItemListSelectionManagerTest::testItemsRemoved() // Remove items 0 to 4 -> selection must be 5 to 10 m_selectionManager->itemsRemoved(KItemRangeList() << KItemRange(0, 5)); selectedItems = m_selectionManager->selectedItems(); + QCOMPARE(selectedItems.count(), 6); for (int i = 5; i <= 10; ++i) { QVERIFY(selectedItems.contains(i)); }