From b3253304a59b85c76f8a70f20ada4139e715a5b3 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 2 Nov 2019 22:52:27 +0100 Subject: [PATCH] Desktop: don't connect to remove() slot of model from TableModel When connecting a model to the TableModel class, it would connect clicking on an item to the remove() slot of the model. This breaks the program flow implied by the undo code: Ui --> Undo-Command --> Model --> UI Moreover, the naming of the remove() slot is illogical, because clicks can also have different effects, as for example in the cylinder-table. Therefore, move the connect() call from TableModel to the callers. In the case of TabDiveSite, move the remove() function from the model to the TabWidget, where it makes more sense. Signed-off-by: Berthold Stoeger --- desktop-widgets/diveplanner.cpp | 2 ++ .../tab-widgets/TabDiveEquipment.cpp | 6 +++-- .../tab-widgets/TabDiveExtraInfo.cpp | 3 +-- desktop-widgets/tab-widgets/TabDiveSite.cpp | 24 ++++++++++++++++++- desktop-widgets/tab-widgets/TabDiveSite.h | 1 + desktop-widgets/tableview.cpp | 3 ++- desktop-widgets/tableview.h | 5 +--- qt-models/divelocationmodel.cpp | 23 ------------------ qt-models/divelocationmodel.h | 2 -- 9 files changed, 34 insertions(+), 35 deletions(-) diff --git a/desktop-widgets/diveplanner.cpp b/desktop-widgets/diveplanner.cpp index 663bb03fb..aac81c81e 100644 --- a/desktop-widgets/diveplanner.cpp +++ b/desktop-widgets/diveplanner.cpp @@ -114,12 +114,14 @@ DivePlannerWidget::DivePlannerWidget(QWidget *parent, Qt::WindowFlags f) : QWidg ui.dateEdit->setDisplayFormat(prefs.date_format); ui.tableWidget->setTitle(tr("Dive planner points")); ui.tableWidget->setModel(plannerModel); + connect(ui.tableWidget, &TableView::itemClicked, plannerModel, &DivePlannerPointsModel::remove); plannerModel->setRecalc(true); ui.tableWidget->view()->setItemDelegateForColumn(DivePlannerPointsModel::GAS, new AirTypesDelegate(this)); ui.tableWidget->view()->setItemDelegateForColumn(DivePlannerPointsModel::DIVEMODE, new DiveTypesDelegate(this)); ui.cylinderTableWidget->setTitle(tr("Available gases")); ui.cylinderTableWidget->setBtnToolTip(tr("Add cylinder")); ui.cylinderTableWidget->setModel(CylindersModel::instance()); + connect(ui.cylinderTableWidget, &TableView::itemClicked, CylindersModel::instance(), &CylindersModel::remove); ui.waterType->setItemData(0, FRESHWATER_SALINITY); ui.waterType->setItemData(1, SEAWATER_SALINITY); ui.waterType->setItemData(2, EN13319_SALINITY); diff --git a/desktop-widgets/tab-widgets/TabDiveEquipment.cpp b/desktop-widgets/tab-widgets/TabDiveEquipment.cpp index 1ea57e703..cb7182e02 100644 --- a/desktop-widgets/tab-widgets/TabDiveEquipment.cpp +++ b/desktop-widgets/tab-widgets/TabDiveEquipment.cpp @@ -33,8 +33,10 @@ TabDiveEquipment::TabDiveEquipment(QWidget *parent) : TabBase(parent), ui.weights->setModel(weightModel); connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &TabDiveEquipment::divesChanged); - connect(ui.cylinders->view(), &QTableView::clicked, this, &TabDiveEquipment::editCylinderWidget); - connect(ui.weights->view(), &QTableView::clicked, this, &TabDiveEquipment::editWeightWidget); + connect(ui.cylinders, &TableView::itemClicked, cylindersModel, &CylindersModel::remove); + connect(ui.cylinders, &TableView::itemClicked, this, &TabDiveEquipment::editCylinderWidget); + connect(ui.weights, &TableView::itemClicked, weightModel, &WeightModel::remove); + connect(ui.weights, &TableView::itemClicked, this, &TabDiveEquipment::editWeightWidget); // Current display of things on Gnome3 looks like shit, so // let's fix that. diff --git a/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp b/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp index cc46fb957..caf7c99bf 100644 --- a/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp +++ b/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp @@ -1,8 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include "TabDiveExtraInfo.h" #include "ui_TabDiveExtraInfo.h" - -#include +#include "qt-models/divecomputerextradatamodel.h" TabDiveExtraInfo::TabDiveExtraInfo(QWidget *parent) : TabBase(parent), diff --git a/desktop-widgets/tab-widgets/TabDiveSite.cpp b/desktop-widgets/tab-widgets/TabDiveSite.cpp index 3ff693e59..9e269ee26 100644 --- a/desktop-widgets/tab-widgets/TabDiveSite.cpp +++ b/desktop-widgets/tab-widgets/TabDiveSite.cpp @@ -4,9 +4,10 @@ #include "core/divesite.h" #include "core/divefilter.h" #include "qt-models/divelocationmodel.h" +#include "desktop-widgets/mainwindow.h" // to place message box #include "commands/command.h" -#include +#include TabDiveSite::TabDiveSite(QWidget *parent) : TabBase(parent) { @@ -25,6 +26,7 @@ TabDiveSite::TabDiveSite(QWidget *parent) : TabBase(parent) ui.diveSites->view()->setColumnHidden(i, true); connect(ui.diveSites, &TableView::addButtonClicked, this, &TabDiveSite::add); + connect(ui.diveSites, &TableView::itemClicked, this, &TabDiveSite::diveSiteClicked); connect(ui.diveSites->view()->selectionModel(), &QItemSelectionModel::selectionChanged, this, &TabDiveSite::selectionChanged); // Subtle: We depend on this slot being executed after the slot in the model. @@ -40,6 +42,26 @@ void TabDiveSite::clear() { } +void TabDiveSite::diveSiteClicked(const QModelIndex &index) +{ + struct dive_site *ds = model.getDiveSite(index); + if (!ds) + return; + switch (index.column()) { + case LocationInformationModel::EDIT: + MainWindow::instance()->editDiveSite(ds); + break; + case LocationInformationModel::REMOVE: + if (ds->dives.nr > 0 && + QMessageBox::warning(MainWindow::instance(), tr("Delete dive site?"), + tr("This dive site has %n dive(s). Do you really want to delete it?\n", "", ds->dives.nr), + QMessageBox::Yes|QMessageBox::No) == QMessageBox::No) + return; + Command::deleteDiveSites(QVector{ds}); + break; + } +} + void TabDiveSite::add() { // This is mighty dirty: We hook into the "dive site added" signal and diff --git a/desktop-widgets/tab-widgets/TabDiveSite.h b/desktop-widgets/tab-widgets/TabDiveSite.h index 5927b08a0..c5d69eecc 100644 --- a/desktop-widgets/tab-widgets/TabDiveSite.h +++ b/desktop-widgets/tab-widgets/TabDiveSite.h @@ -16,6 +16,7 @@ private slots: void add(); void diveSiteAdded(struct dive_site *, int idx); void diveSiteChanged(struct dive_site *ds, int field); + void diveSiteClicked(const QModelIndex &); void on_purgeUnused_clicked(); void on_filterText_textChanged(const QString &text); void selectionChanged(const QItemSelection &selected, const QItemSelection &deselected); diff --git a/desktop-widgets/tableview.cpp b/desktop-widgets/tableview.cpp index 560f98a43..13f7a9d03 100644 --- a/desktop-widgets/tableview.cpp +++ b/desktop-widgets/tableview.cpp @@ -10,6 +10,8 @@ TableView::TableView(QWidget *parent) : QGroupBox(parent) ui.setupUi(this); ui.tableView->setItemDelegate(new DiveListDelegate(this)); + connect(ui.tableView, &QTableView::clicked, this, &TableView::itemClicked); + QFontMetrics fm(defaultModelFont()); int text_ht = fm.height(); @@ -93,7 +95,6 @@ void TableView::setBtnToolTip(const QString &tooltip) void TableView::setModel(QAbstractItemModel *model) { ui.tableView->setModel(model); - connect(ui.tableView, SIGNAL(clicked(QModelIndex)), model, SLOT(remove(QModelIndex))); QSettings s; s.beginGroup(objectName()); diff --git a/desktop-widgets/tableview.h b/desktop-widgets/tableview.h index 5dc5db2b5..7b783ef02 100644 --- a/desktop-widgets/tableview.h +++ b/desktop-widgets/tableview.h @@ -28,10 +28,6 @@ class TableView : public QGroupBox { public: TableView(QWidget *parent = 0); ~TableView(); - /* The model is expected to have a 'remove' slot, that takes a QModelIndex as parameter. - * It's also expected to have the column '1' as a trash icon. I most probably should create a - * proxy model and add that column, will mark that as TODO. see? marked. - */ void setModel(QAbstractItemModel *model); void setBtnToolTip(const QString &tooltip); void fixPlusPosition(); @@ -45,6 +41,7 @@ protected: signals: void addButtonClicked(); + void itemClicked(const QModelIndex &); private: Ui::TableView ui; diff --git a/qt-models/divelocationmodel.cpp b/qt-models/divelocationmodel.cpp index f68209a7e..0be755fd2 100644 --- a/qt-models/divelocationmodel.cpp +++ b/qt-models/divelocationmodel.cpp @@ -7,9 +7,7 @@ #include "core/metrics.h" #ifndef SUBSURFACE_MOBILE #include "cleanertablemodel.h" // for trashIcon() and editIcon() -#include "desktop-widgets/mainwindow.h" // to place message box #include "commands/command.h" -#include #endif #include #include @@ -262,27 +260,6 @@ bool DiveSiteSortedModel::setData(const QModelIndex &index, const QVariant &valu } } -// TODO: Remove or edit. It doesn't make sense to call the model here, which calls the undo command, -// which in turn calls the model. -void DiveSiteSortedModel::remove(const QModelIndex &index) -{ - struct dive_site *ds = getDiveSite(index); - if (!ds) - return; - switch (index.column()) { - case LocationInformationModel::EDIT: - MainWindow::instance()->editDiveSite(ds); - break; - case LocationInformationModel::REMOVE: - if (ds->dives.nr > 0 && - QMessageBox::warning(MainWindow::instance(), tr("Delete dive site?"), - tr("This dive site has %n dive(s). Do you really want to delete it?\n", "", ds->dives.nr), - QMessageBox::Yes|QMessageBox::No) == QMessageBox::No) - return; - Command::deleteDiveSites(QVector{ds}); - break; - } -} #endif // SUBSURFACE_MOBILE void DiveSiteSortedModel::setFilter(const QString &text) diff --git a/qt-models/divelocationmodel.h b/qt-models/divelocationmodel.h index a42354737..661debc53 100644 --- a/qt-models/divelocationmodel.h +++ b/qt-models/divelocationmodel.h @@ -46,8 +46,6 @@ private: QString fullText; #ifndef SUBSURFACE_MOBILE bool setData(const QModelIndex &index, const QVariant &value, int role) override; -public slots: - void remove(const QModelIndex &index); #endif // SUBSURFACE_MOBILE public: DiveSiteSortedModel();