From 7504c966bf64a4b38b8e9ff200c93b2e8470acad Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Thu, 2 Jul 2020 14:49:26 +0200 Subject: [PATCH] QmlDesigner: Move code from list editor dialog to model And add tests to the code. Change-Id: I9fb183729c716a50bbab861d207a212ff704ee7b Reviewed-by: Tim Jenssen --- .../listmodeleditor/listmodeleditordialog.cpp | 32 +--- .../listmodeleditor/listmodeleditormodel.cpp | 54 +++++++ .../listmodeleditor/listmodeleditormodel.h | 11 +- tests/unit/unittest/listmodeleditor-test.cpp | 148 ++++++++++++++---- 4 files changed, 181 insertions(+), 64 deletions(-) diff --git a/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditordialog.cpp b/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditordialog.cpp index 283acab78bb..968c719af08 100644 --- a/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditordialog.cpp +++ b/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditordialog.cpp @@ -114,40 +114,12 @@ void ListModelEditorDialog::openColumnDialog() void ListModelEditorDialog::removeRows() { - const QList indices = m_tableView->selectionModel()->selectedRows(); - std::vector rows; - rows.reserve(indices.size()); - - for (QModelIndex index : indices) - rows.push_back(index.row()); - - std::sort(rows.begin(), rows.end()); - - rows.erase(std::unique(rows.begin(), rows.end()), rows.end()); - - std::reverse(rows.begin(), rows.end()); - - for (int row : rows) - m_model->removeRow(row); + m_model->removeRows(m_tableView->selectionModel()->selectedRows()); } void ListModelEditorDialog::removeColumns() { - const QList indices = m_tableView->selectionModel()->selectedColumns(); - std::vector columns; - columns.reserve(indices.size()); - - for (QModelIndex index : indices) - columns.push_back(index.column()); - - std::sort(columns.begin(), columns.end()); - - columns.erase(std::unique(columns.begin(), columns.end()), columns.end()); - - std::reverse(columns.begin(), columns.end()); - - for (int row : columns) - m_model->removeColumn(row); + m_model->removeColumns(m_tableView->selectionModel()->selectedColumns()); } void ListModelEditorDialog::changeHeader(int column) diff --git a/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.cpp b/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.cpp index 98722c3e8fb..066af6e3451 100644 --- a/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.cpp +++ b/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.cpp @@ -260,6 +260,26 @@ void ListModelEditorModel::removeColumn(int column) } } +void ListModelEditorModel::removeColumns(const QList &indices) +{ + std::vector columns = filterColumns(indices); + + std::reverse(columns.begin(), columns.end()); + + for (int column : columns) + removeColumn(column); +} + +void ListModelEditorModel::removeRows(const QList &indices) +{ + std::vector rows = filterRows(indices); + + std::reverse(rows.begin(), rows.end()); + + for (int row : rows) + removeRow(row); +} + void ListModelEditorModel::removeRow(int row) { QList rowItems = QStandardItemModel::takeRow(row); @@ -299,4 +319,38 @@ void ListModelEditorModel::renameColumn(int oldColumn, const QString &newColumnN setHorizontalHeaderLabels(convertToStringList(m_propertyNames)); } +std::vector ListModelEditorModel::filterColumns(const QList &indices) +{ + std::vector columns; + columns.reserve(indices.size()); + + for (QModelIndex index : indices) { + if (index.column() >= 0) + columns.push_back(index.column()); + } + + std::sort(columns.begin(), columns.end()); + + columns.erase(std::unique(columns.begin(), columns.end()), columns.end()); + + return columns; +} + +std::vector ListModelEditorModel::filterRows(const QList &indices) +{ + std::vector rows; + rows.reserve(indices.size()); + + for (QModelIndex index : indices) { + if (index.row() >= 0) + rows.push_back(index.row()); + } + + std::sort(rows.begin(), rows.end()); + + rows.erase(std::unique(rows.begin(), rows.end()), rows.end()); + + return rows; +} + } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.h b/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.h index 35d41bee68b..3976b34a9d8 100644 --- a/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.h +++ b/src/plugins/qmldesigner/components/listmodeleditor/listmodeleditormodel.h @@ -33,6 +33,8 @@ namespace QmlDesigner { class ListModelEditorModel : public QStandardItemModel { + using QStandardItemModel::removeColumns; + using QStandardItemModel::removeRows; public: void setListModel(ModelNode node) @@ -48,11 +50,16 @@ public: bool setValue(int row, int column, QVariant value, Qt::ItemDataRole role = Qt::EditRole); - void removeColumn(int column); - void removeRow(int row); + void removeColumns(const QList &indices); + void removeRows(const QList &indices); void renameColumn(int column, const QString &newColumnName); + static std::vector filterColumns(const QList &indices); + static std::vector filterRows(const QList &indices); + private: + void removeRow(int row); + void removeColumn(int column); void populateModel(); void createItems(const QList &listElementNodes); void appendItems(const ModelNode &listElementNode); diff --git a/tests/unit/unittest/listmodeleditor-test.cpp b/tests/unit/unittest/listmodeleditor-test.cpp index ca0913f8656..1dcd54c2868 100644 --- a/tests/unit/unittest/listmodeleditor-test.cpp +++ b/tests/unit/unittest/listmodeleditor-test.cpp @@ -39,6 +39,7 @@ namespace { using QmlDesigner::AbstractProperty; using QmlDesigner::AbstractView; +using QmlDesigner::ListModelEditorModel; using QmlDesigner::ModelNode; MATCHER_P2(HasItem, @@ -172,6 +173,8 @@ public: return properties; } + QModelIndex index(int row, int column) const { return model.index(row, column); } + protected: std::unique_ptr designerModel{QmlDesigner::Model::create("QtQuick.Item", 1, 1)}; NiceMock mockView; @@ -427,7 +430,7 @@ TEST_F(ListModelEditor, RemoveColumnRemovesDisplayValues) { model.setListModel(listModelNode); - model.removeColumn(2); + model.removeColumns({index(0, 2)}); ASSERT_THAT(displayValues(), ElementsAre(ElementsAre(IsInvalid(), "foo", 42), @@ -442,14 +445,14 @@ TEST_F(ListModelEditor, RemoveColumnRemovesProperties) EXPECT_CALL(mockView, propertiesRemoved(ElementsAre(IsAbstractProperty(element2, "image")))); EXPECT_CALL(mockView, propertiesRemoved(ElementsAre(IsAbstractProperty(element3, "image")))); - model.removeColumn(0); + model.removeColumns({index(0, 0)}); } TEST_F(ListModelEditor, RemoveColumnRemovesPropertyName) { model.setListModel(listModelNode); - model.removeColumn(1); + model.removeColumns({index(0, 1)}); ASSERT_THAT(model.propertyNames(), ElementsAre("image", "value", "value2")); } @@ -458,7 +461,7 @@ TEST_F(ListModelEditor, RemoveRowRemovesDisplayValues) { model.setListModel(listModelNode); - model.removeRow(1); + model.removeRows({index(1, 0)}); ASSERT_THAT(displayValues(), ElementsAre(ElementsAre(IsInvalid(), "foo", 1, 42), @@ -471,7 +474,7 @@ TEST_F(ListModelEditor, RemoveRowRemovesElementInListModel) EXPECT_CALL(mockView, nodeRemoved(Eq(element2), _, _)); - model.removeRow(1); + model.removeRows({index(1, 0)}); } TEST_F(ListModelEditor, ConvertStringFloatToFloat) @@ -721,7 +724,7 @@ TEST_F(ListModelEditor, RemoveColumnAfterRenameColumn) model.setListModel(listModelNode); model.renameColumn(1, "mood"); - model.removeColumn(1); + model.removeColumns({index(0, 1)}); ASSERT_THAT(properties(), ElementsAre(UnorderedElementsAre(IsVariantProperty("value", 1), @@ -909,30 +912,7 @@ TEST_F(ListModelEditor, RemoveLastRow) model.addColumn("mood"); model.addRow(); - model.removeRow(0); - - ASSERT_THAT(displayValues(), IsEmpty()); -} - -TEST_F(ListModelEditor, RemoveLastColumn) -{ - model.setListModel(emptyListModelNode); - model.addColumn("mood"); - model.addRow(); - - model.removeColumn(0); - - ASSERT_THAT(displayValues(), ElementsAre(IsEmpty())); -} - -TEST_F(ListModelEditor, RemoveLastEmptyColumn) -{ - model.setListModel(emptyListModelNode); - model.addColumn("mood"); - model.addRow(); - model.removeRow(0); - - model.removeColumn(0); + model.removeRows({index(0, 0)}); ASSERT_THAT(displayValues(), IsEmpty()); } @@ -942,11 +922,115 @@ TEST_F(ListModelEditor, RemoveLastEmptyRow) model.setListModel(emptyListModelNode); model.addColumn("mood"); model.addRow(); - model.removeColumn(0); + model.removeColumns({index(0, 0)}); - model.removeRow(0); + model.removeRows({index(0, 0)}); + + ASSERT_THAT(displayValues(), ElementsAre(IsEmpty())); +} + +TEST_F(ListModelEditor, RemoveLastColumn) +{ + model.setListModel(emptyListModelNode); + model.addColumn("mood"); + model.addRow(); + + model.removeColumns({index(0, 0)}); + + ASSERT_THAT(displayValues(), ElementsAre(IsEmpty())); +} + +TEST_F(ListModelEditor, RemoveLastEmptyColumn) +{ + model.setListModel(emptyListModelNode); + model.addColumn("mood"); + model.addRow(); + model.removeRows({index(0, 0)}); + + model.removeColumns({index(0, 0)}); ASSERT_THAT(displayValues(), IsEmpty()); } +TEST_F(ListModelEditor, RemoveColumns) +{ + model.setListModel(listModelNode); + model.removeColumns({index(0, 1), index(0, 3), index(1, 1), index(0, 4)}); + + ASSERT_THAT(properties(), + ElementsAre(UnorderedElementsAre(IsVariantProperty("value", 1)), + UnorderedElementsAre(IsVariantProperty("image", "pic.png"), + IsVariantProperty("value", 4)), + UnorderedElementsAre(IsVariantProperty("image", "pic.png"), + IsVariantProperty("value", 111)))); +} + +TEST_F(ListModelEditor, RemoveRows) +{ + model.setListModel(listModelNode); + + model.removeRows({index(1, 0), index(2, 0), index(3, 0), index(2, 0)}); + + ASSERT_THAT(properties(), + ElementsAre(UnorderedElementsAre(IsVariantProperty("name", "foo"), + IsVariantProperty("value", 1), + IsVariantProperty("value2", 42)))); +} + +TEST_F(ListModelEditor, FilterColumns) +{ + model.setListModel(listModelNode); + QList indices = {index(0, 0), index(1, 1), index(0, 2), index(0, 1)}; + + auto columns = ListModelEditorModel::filterColumns(indices); + + ASSERT_THAT(columns, ElementsAre(0, 1, 2)); +} + +TEST_F(ListModelEditor, FilterColumnsInvalidColumns) +{ + QList indices = {index(0, 0), index(1, 1), index(0, 2), index(0, 1)}; + + auto columns = ListModelEditorModel::filterColumns(indices); + + ASSERT_THAT(columns, IsEmpty()); +} + +TEST_F(ListModelEditor, FilterColumnsEmptyInput) +{ + QList indices; + + auto columns = ListModelEditorModel::filterColumns(indices); + + ASSERT_THAT(columns, IsEmpty()); +} + +TEST_F(ListModelEditor, FilterRows) +{ + model.setListModel(listModelNode); + QList indices = {index(0, 0), index(1, 1), index(2, 2), index(0, 1)}; + + auto rows = ListModelEditorModel::filterRows(indices); + + ASSERT_THAT(rows, ElementsAre(0, 1, 2)); +} + +TEST_F(ListModelEditor, FilterRowsInvalidColumns) +{ + QList indices = {index(0, 0), index(1, 1), index(2, 2), index(0, 1)}; + + auto rows = ListModelEditorModel::filterRows(indices); + + ASSERT_THAT(rows, IsEmpty()); +} + +TEST_F(ListModelEditor, FilterRowsEmptyInput) +{ + QList indices; + + auto rows = ListModelEditorModel::filterRows(indices); + + ASSERT_THAT(rows, IsEmpty()); +} + } // namespace