diff options
author | Luca Beldi <v.ronin@yahoo.it> | 2018-09-06 09:08:31 +0100 |
---|---|---|
committer | Luca Beldi <v.ronin@yahoo.it> | 2018-09-06 09:27:09 +0000 |
commit | 37a1c6dc4c102753b20b79b9460c03ad6c6ae08e (patch) | |
tree | 50cdb10450d1d5d90f3a50e5ebe4683976e55ecc | |
parent | fcdb459c06c0756637e3301430290df5560494e0 (diff) |
Reimplement QStringListModel::setItemData
Before this patch QStringListModel::setItemData emitted dataChanged
twice if the roles map contained both DisplayRole and EditRole.
This fixes the duplication.
Increased efficiency of QStringListModel::itemData
Task-number: QTBUG-67511
Change-Id: Ibaea17530f15627a3cb8003e5284e54001731ded
Reviewed-by: David Faure <david.faure@kdab.com>
-rw-r--r-- | src/corelib/itemmodels/qstringlistmodel.cpp | 44 | ||||
-rw-r--r-- | src/corelib/itemmodels/qstringlistmodel.h | 3 | ||||
-rw-r--r-- | tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp | 71 |
3 files changed, 111 insertions, 7 deletions
diff --git a/src/corelib/itemmodels/qstringlistmodel.cpp b/src/corelib/itemmodels/qstringlistmodel.cpp index 7a5ed1c317..3cc0bee8ef 100644 --- a/src/corelib/itemmodels/qstringlistmodel.cpp +++ b/src/corelib/itemmodels/qstringlistmodel.cpp @@ -136,6 +136,42 @@ QModelIndex QStringListModel::sibling(int row, int column, const QModelIndex &id } /*! + \reimp + \since 5.13 +*/ +QMap<int, QVariant> QStringListModel::itemData(const QModelIndex &index) const +{ + if (!checkIndex(index, CheckIndexOption::IndexIsValid | CheckIndexOption::ParentIsInvalid)) + return QMap<int, QVariant>{}; + const QVariant displayData = lst.at(index.row()); + return QMap<int, QVariant>{{ + std::make_pair<int>(Qt::DisplayRole, displayData), + std::make_pair<int>(Qt::EditRole, displayData) + }}; +} + +/*! + \reimp + \since 5.13 + If \a roles contains both Qt::DisplayRole and Qt::EditRole, the latter will take precedence +*/ +bool QStringListModel::setItemData(const QModelIndex &index, const QMap<int, QVariant> &roles) +{ + if (roles.isEmpty()) + return false; + if (std::any_of(roles.keyBegin(), roles.keyEnd(), [](int role) -> bool { + return role != Qt::DisplayRole && role != Qt::EditRole; + })) { + return false; + } + auto roleIter = roles.constFind(Qt::EditRole); + if (roleIter == roles.constEnd()) + roleIter = roles.constFind(Qt::DisplayRole); + Q_ASSERT(roleIter != roles.constEnd()); + return setData(index, roleIter.value(), roleIter.key()); +} + +/*! Returns data for the specified \a role, from the item with the given \a index. @@ -185,13 +221,7 @@ bool QStringListModel::setData(const QModelIndex &index, const QVariant &value, if (index.row() >= 0 && index.row() < lst.size() && (role == Qt::EditRole || role == Qt::DisplayRole)) { lst.replace(index.row(), value.toString()); - QVector<int> roles; - roles.reserve(2); - roles.append(Qt::DisplayRole); - roles.append(Qt::EditRole); - emit dataChanged(index, index, roles); - // once Q_COMPILER_UNIFORM_INIT can be used, change to: - // emit dataChanged(index, index, {Qt::DisplayRole, Qt::EditRole}); + emit dataChanged(index, index, {Qt::DisplayRole, Qt::EditRole}); return true; } return false; diff --git a/src/corelib/itemmodels/qstringlistmodel.h b/src/corelib/itemmodels/qstringlistmodel.h index 1ad8ea0adf..6c83917054 100644 --- a/src/corelib/itemmodels/qstringlistmodel.h +++ b/src/corelib/itemmodels/qstringlistmodel.h @@ -69,6 +69,9 @@ public: bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex()) override; bool moveRows(const QModelIndex &sourceParent, int sourceRow, int count, const QModelIndex &destinationParent, int destinationChild) override; + QMap<int, QVariant> itemData(const QModelIndex &index) const override; + bool setItemData(const QModelIndex &index, const QMap<int, QVariant> &roles) override; + void sort(int column, Qt::SortOrder order = Qt::AscendingOrder) override; QStringList stringList() const; diff --git a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp index e9272cbf92..16e5170a47 100644 --- a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp +++ b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp @@ -87,6 +87,9 @@ private slots: void moveRows(); void moveRowsInvalid_data(); void moveRowsInvalid(); + + void itemData(); + void setItemData(); }; void tst_QStringListModel::moveRowsInvalid_data() @@ -351,6 +354,74 @@ void tst_QStringListModel::setData_emits_both_roles() expected); } +void tst_QStringListModel::itemData() +{ + QStringListModel testModel{ QStringList { + QStringLiteral("One"), + QStringLiteral("Two"), + QStringLiteral("Three"), + QStringLiteral("Four"), + QStringLiteral("Five") + }}; + QMap<int, QVariant> compareMap; + QCOMPARE(testModel.itemData(QModelIndex()), compareMap); + compareMap.insert(Qt::DisplayRole, QStringLiteral("Two")); + compareMap.insert(Qt::EditRole, QStringLiteral("Two")); + QCOMPARE(testModel.itemData(testModel.index(1, 0)), compareMap); +} + +void tst_QStringListModel::setItemData() +{ + QStringListModel testModel{ QStringList { + QStringLiteral("One"), + QStringLiteral("Two"), + QStringLiteral("Three"), + QStringLiteral("Four"), + QStringLiteral("Five") + }}; + QSignalSpy dataChangedSpy(&testModel, &QAbstractItemModel::dataChanged); + QModelIndex changeIndex = testModel.index(1, 0); + const QVector<int> changeRoles{Qt::DisplayRole, Qt::EditRole}; + const QString changedString("Changed"); + QMap<int, QVariant> newItemData{std::make_pair<int>(Qt::DisplayRole, changedString)}; + // invalid index does nothing and returns false + QVERIFY(!testModel.setItemData(QModelIndex(), newItemData)); + // valid data is set, return value is true and dataChanged is emitted once + QVERIFY(testModel.setItemData(changeIndex, newItemData)); + QCOMPARE(changeIndex.data(Qt::DisplayRole).toString(), changedString); + QCOMPARE(changeIndex.data(Qt::EditRole).toString(), changedString); + QCOMPARE(dataChangedSpy.size(), 1); + QVariantList dataChangedArguments = dataChangedSpy.takeFirst(); + QCOMPARE(dataChangedArguments.at(0).value<QModelIndex>(), changeIndex); + QCOMPARE(dataChangedArguments.at(1).value<QModelIndex>(), changeIndex); + QCOMPARE(dataChangedArguments.at(2).value<QVector<int> >(), changeRoles); + // Unsupported roles do nothing return false + newItemData.clear(); + newItemData.insert(Qt::UserRole, changedString); + QVERIFY(!testModel.setItemData(changeIndex, newItemData)); + QCOMPARE(dataChangedSpy.size(), 0); + // If some but not all the roles are supported it returns false and does nothing + newItemData.insert(Qt::EditRole, changedString); + changeIndex = testModel.index(2, 0); + QVERIFY(!testModel.setItemData(changeIndex, newItemData)); + QCOMPARE(changeIndex.data(Qt::DisplayRole).toString(), QStringLiteral("Three")); + QCOMPARE(changeIndex.data(Qt::EditRole).toString(), QStringLiteral("Three")); + QCOMPARE(dataChangedSpy.size(), 0); + // Qt::EditRole and Qt::DisplayRole are both set, Qt::EditRole takes precedence + newItemData.clear(); + newItemData.insert(Qt::EditRole, changedString); + newItemData.insert(Qt::DisplayRole, QStringLiteral("Ignored")); + changeIndex = testModel.index(3, 0); + QVERIFY(testModel.setItemData(changeIndex, newItemData)); + QCOMPARE(changeIndex.data(Qt::DisplayRole).toString(), changedString); + QCOMPARE(changeIndex.data(Qt::EditRole).toString(), changedString); + QCOMPARE(dataChangedSpy.size(), 1); + dataChangedArguments = dataChangedSpy.takeFirst(); + QCOMPARE(dataChangedArguments.at(0).value<QModelIndex>(), changeIndex); + QCOMPARE(dataChangedArguments.at(1).value<QModelIndex>(), changeIndex); + QCOMPARE(dataChangedArguments.at(2).value<QVector<int> >(), changeRoles); +} + void tst_QStringListModel::supportedDragDropActions() { QStringListModel model; |