From 8581e213ed2d5604ade8130cacfd7168db172d26 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 27 Aug 2022 17:28:34 +0200 Subject: [PATCH] selection: trickle down trip selection The trip selection code was an awkward layering violation. Whereas dive selections due to dive undo-commands trickled down via DiveTripModel-->MultiFilterSortModel-->DiveListView, for trip editing, the DiveListView directly intercepted the TripEdited signal. Instead, mimic the dive-selection code. This is a bit longer but more consistent and logical. The undo/redo of trip changes is now also a "programmatical" change of the selection. Signed-off-by: Berthold Stoeger --- commands/command_edit_trip.cpp | 2 + commands/command_edit_trip.h | 1 + core/selection.cpp | 34 +++++++----- core/selection.h | 3 ++ core/subsurface-qt/divelistnotifier.h | 1 + desktop-widgets/divelistview.cpp | 74 ++++++++++++--------------- desktop-widgets/divelistview.h | 4 +- qt-models/divetripmodel.cpp | 35 +++++++++++++ qt-models/divetripmodel.h | 3 ++ qt-models/filtermodels.cpp | 11 ++++ qt-models/filtermodels.h | 2 + 11 files changed, 113 insertions(+), 57 deletions(-) diff --git a/commands/command_edit_trip.cpp b/commands/command_edit_trip.cpp index f6d7251ea..64f5d05a0 100644 --- a/commands/command_edit_trip.cpp +++ b/commands/command_edit_trip.cpp @@ -7,6 +7,7 @@ namespace Command { EditTripBase::EditTripBase(dive_trip *tripIn, const QString &newValue) : trip(tripIn), + current(current_dive), value(newValue) { } @@ -26,6 +27,7 @@ void EditTripBase::undo() value = old; emit diveListNotifier.tripChanged(trip, fieldId()); + setTripSelection(trip, current); } // Undo and redo do the same as just the stored value is exchanged diff --git a/commands/command_edit_trip.h b/commands/command_edit_trip.h index f1d633b4b..778eb0208 100644 --- a/commands/command_edit_trip.h +++ b/commands/command_edit_trip.h @@ -20,6 +20,7 @@ class EditTripBase : public Base { bool workToBeDone() override; dive_trip *trip; // Trip to be edited. + dive *current; public: EditTripBase(dive_trip *trip, const QString &newValue); diff --git a/core/selection.cpp b/core/selection.cpp index 09f097e0f..fc48fdf59 100644 --- a/core/selection.cpp +++ b/core/selection.cpp @@ -224,6 +224,27 @@ bool setSelectionKeepCurrent(const std::vector &selection) return current_dive != oldCurrent; } +void setTripSelection(dive_trip *trip, dive *currentDive) +{ + if (!trip) + return; + + current_dive = currentDive; + for (int i = 0; i < divelog.dives->nr; ++i) { + dive &d = *divelog.dives->dives[i]; + d.selected = d.divetrip == trip; + } + for (int i = 0; i < divelog.trips->nr; ++i) { + dive_trip *t = divelog.trips->trips[i]; + t->selected = t == trip; + } + + amount_selected = trip->dives.nr; + amount_trips_selected = 1; + + emit diveListNotifier.tripSelected(trip, currentDive); +} + extern "C" void select_single_dive(dive *d) { if (d) @@ -314,16 +335,3 @@ extern "C" struct dive_trip *single_selected_trip() fprintf(stderr, "warning: found no selected trip even though one should be selected\n"); return NULL; // shouldn't happen } - -extern "C" void clear_selection(void) -{ - current_dive = nullptr; - amount_selected = 0; - amount_trips_selected = 0; - int i; - struct dive *dive; - for_each_dive (i, dive) - dive->selected = false; - for (int i = 0; i < divelog.trips->nr; ++i) - divelog.trips->trips[i]->selected = false; -} diff --git a/core/selection.h b/core/selection.h index dd83f6655..7c579cf0d 100644 --- a/core/selection.h +++ b/core/selection.h @@ -57,6 +57,9 @@ void setSelection(const std::vector &selection, dive *currentDive, int c // Does not send a signal. bool setSelectionKeepCurrent(const std::vector &selection); +// Select all dive in a trip and sends a trip-selected signal +void setTripSelection(dive_trip *trip, dive *currentDive); + // Get currently selected dives std::vector getDiveSelection(); bool diveInSelection(const std::vector &selection, const dive *d); diff --git a/core/subsurface-qt/divelistnotifier.h b/core/subsurface-qt/divelistnotifier.h index 7bbbdfbfb..0612d229f 100644 --- a/core/subsurface-qt/divelistnotifier.h +++ b/core/subsurface-qt/divelistnotifier.h @@ -112,6 +112,7 @@ signals: // Selection changes void divesSelected(const QVector &dives, dive *currentDive); + void tripSelected(dive_trip *trip, dive *currentDive); // Dive site signals. Add and delete events are sent per dive site and // provide an index into the global dive site table. diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp index 1346b0be7..57da7e2c3 100644 --- a/desktop-widgets/divelistview.cpp +++ b/desktop-widgets/divelistview.cpp @@ -40,6 +40,7 @@ DiveListView::DiveListView(QWidget *parent) : QTreeView(parent), MultiFilterSortModel *m = MultiFilterSortModel::instance(); setModel(m); connect(m, &MultiFilterSortModel::selectionChanged, this, &DiveListView::diveSelectionChanged); + connect(m, &MultiFilterSortModel::tripSelected, this, &DiveListView::tripSelected); connect(&diveListNotifier, &DiveListNotifier::settingsChanged, this, &DiveListView::settingsChanged); setSortingEnabled(true); @@ -49,8 +50,6 @@ DiveListView::DiveListView(QWidget *parent) : QTreeView(parent), resetModel(); - connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &DiveListView::tripChanged); - header()->setStretchLastSection(true); header()->setSortIndicatorShown(true); header()->setSectionsClickable(true); @@ -286,42 +285,28 @@ void DiveListView::rowsInserted(const QModelIndex &parent, int start, int end) } } -// This is a bit ugly: we hook directly into the tripChanged signal to -// select the trip if it was edited. This feels like a layering violation: -// Shouldn't the core-layer call us? -void DiveListView::tripChanged(dive_trip *trip, TripField) +void DiveListView::tripSelected(QModelIndex trip, QModelIndex currentDive) { - // First check if the trip is already selected (and only this trip, as only then is it displayed). - if (single_selected_trip() == trip) - return; + programmaticalSelectionChange = true; - unselectDives(); - selectTrip(trip); - selectionChangeDone(); -} - -void DiveListView::selectTrip(dive_trip_t *trip) -{ - if (!trip) - return; - - QAbstractItemModel *m = model(); - QModelIndexList match = m->match(m->index(0, 0), DiveTripModelBase::TRIP_ROLE, QVariant::fromValue(trip), 2, Qt::MatchRecursive); - QItemSelectionModel::SelectionFlags flags; - if (!match.count()) - return; - QModelIndex idx = match.first(); - flags = QItemSelectionModel::Select; - flags |= QItemSelectionModel::Rows; - selectionModel()->select(idx, flags); - expand(idx); -} - -void DiveListView::unselectDives() -{ - clear_selection(); // clear the Qt selection selectionModel()->clearSelection(); + + if (trip.isValid()) { + QItemSelectionModel::SelectionFlags flags = QItemSelectionModel::Select | QItemSelectionModel::Rows; + selectionModel()->select(trip, flags); + selectTripItems(trip); + } + + // Set the currently activated row. + // Note, we have to use the QItemSelectionModel::Current mode to avoid + // changing our selection (in contrast to Qt's documentation, which + // instructs to use QItemSelectionModel::NoUpdate, which results in + // funny side-effects). + selectionModel()->setCurrentIndex(currentDive, QItemSelectionModel::Current); + + selectionChangeDone(); + programmaticalSelectionChange = false; } bool DiveListView::eventFilter(QObject *, QEvent *event) @@ -527,6 +512,18 @@ void DiveListView::selectionChangeDone() emit divesSelected(); } +void DiveListView::selectTripItems(QModelIndex index) +{ + const QAbstractItemModel *model = index.model(); + if (model->rowCount(index)) { + QItemSelection selection; + selection.select(model->index(0, 0, index), model->index(model->rowCount(index) - 1, 0, index)); + selectionModel()->select(selection, QItemSelectionModel::Select | QItemSelectionModel::Rows); + if (!isExpanded(index)) + expand(index); + } +} + void DiveListView::selectionChanged(const QItemSelection &selected, const QItemSelection &deselected) { if (programmaticalSelectionChange) { @@ -570,14 +567,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS select_trip(trip); for (int i = 0; i < trip->dives.nr; ++i) addToSelection.push_back(trip->dives.dives[i]); - if (model->rowCount(index)) { - QItemSelection selection; - selection.select(model->index(0, 0, index), model->index(model->rowCount(index) - 1, 0, index)); - selectionModel()->select(selection, QItemSelectionModel::Select | QItemSelectionModel::Rows); - selectionModel()->setCurrentIndex(index, QItemSelectionModel::Select | QItemSelectionModel::NoUpdate); - if (!isExpanded(index)) - expand(index); - } + selectTripItems(index); } } diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h index e46abdcf4..b16a23067 100644 --- a/desktop-widgets/divelistview.h +++ b/desktop-widgets/divelistview.h @@ -52,17 +52,17 @@ slots: void addDivesToTrip(); void shiftTimes(); void diveSelectionChanged(const QVector &indices, QModelIndex currentDive); - void tripChanged(dive_trip *trip, TripField); + void tripSelected(QModelIndex trip, QModelIndex currentDive); private: void rowsInserted(const QModelIndex &parent, int start, int end) override; void reset() override; void setSelection(const QRect &rect, QItemSelectionModel::SelectionFlags flags) override; - void unselectDives(); void mouseReleaseEvent(QMouseEvent *event) override; void keyPressEvent(QKeyEvent *event) override; void selectAll() override; void selectionChanged(const QItemSelection &selected, const QItemSelection &deselected) override; void selectionChangeDone(); + void selectTripItems(QModelIndex index); DiveTripModelBase::Layout currentLayout; QModelIndex contextMenuIndex; // Remember the initial column widths, to avoid writing unchanged widths to the settings diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 3ed42d864..8870ec5b4 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -683,6 +683,7 @@ DiveTripModelTree::DiveTripModelTree(QObject *parent) : DiveTripModelBase(parent connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModelTree::divesMovedBetweenTrips); connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModelTree::divesTimeChanged); connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &DiveTripModelTree::divesSelected); + connect(&diveListNotifier, &DiveListNotifier::tripSelected, this, &DiveTripModelTree::tripSelected); connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &DiveTripModelTree::tripChanged); connect(&diveListNotifier, &DiveListNotifier::filterReset, this, &DiveTripModelTree::filterReset); connect(&diveListNotifier, &DiveListNotifier::cylinderAdded, this, &DiveTripModelTree::diveChanged); @@ -1425,6 +1426,24 @@ void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector } } +void DiveTripModelTree::tripSelected(dive_trip *trip, dive *currentDive) +{ + if (!trip) + return; + + // Find the trip. + int idx = findTripIdx(trip); + if (idx < 0) { + // We don't know the trip - this shouldn't happen. We seem to have + // missed some signals! + qWarning() << "DiveTripModelTree::tripSelected(): unknown trip"; + return; + } + + QModelIndex tripIdx = createIndex(idx, 0, noParent); + emit DiveTripModelBase::tripSelected(tripIdx, diveToIdx(currentDive)); +} + bool DiveTripModelTree::lessThan(const QModelIndex &i1, const QModelIndex &i2) const { // In tree mode we don't support any sorting! @@ -1445,6 +1464,7 @@ DiveTripModelList::DiveTripModelList(QObject *parent) : DiveTripModelBase(parent //connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModelList::divesMovedBetweenTrips); connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModelList::divesTimeChanged); connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &DiveTripModelList::divesSelected); + connect(&diveListNotifier, &DiveListNotifier::tripSelected, this, &DiveTripModelList::tripSelected); connect(&diveListNotifier, &DiveListNotifier::filterReset, this, &DiveTripModelList::filterReset); connect(&diveListNotifier, &DiveListNotifier::cylinderAdded, this, &DiveTripModelList::diveChanged); connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &DiveTripModelList::diveChanged); @@ -1665,6 +1685,21 @@ void DiveTripModelList::divesSelected(const QVector &divesIn, dive *curr currentChanged(currentDive); } +void DiveTripModelList::tripSelected(dive_trip *trip, dive *currentDive) +{ + if (!trip) + return; + + // In the list view, there are no trips, so simply transform this into + // a dive selection. + QVector dives; + dives.reserve(trip->dives.nr); + for (int i = 0; i < trip->dives.nr; ++i) + dives.push_back(trip->dives.dives[i]); + + divesSelected(dives, currentDive); +} + // Simple sorting helper for sorting against a criterium and if // that is undefined against a different criterium. // Return true if diff1 < 0, false if diff1 > 0. diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h index e2bee4a47..3b3e6c711 100644 --- a/qt-models/divetripmodel.h +++ b/qt-models/divetripmodel.h @@ -85,6 +85,7 @@ signals: // indices into local indices according to current sorting/filtering and instructs the QSelectionModel to // perform the appropriate actions. void selectionChanged(const QVector &indices, QModelIndex currentDive); + void tripSelected(QModelIndex trip, QModelIndex currentDive); protected: dive *oldCurrent; QBrush invalidForeground; @@ -116,6 +117,7 @@ public slots: void diveChanged(dive *d); void divesTimeChanged(timestamp_t delta, const QVector &dives); void divesSelected(const QVector &dives, dive *currentDive); + void tripSelected(dive_trip *trip, dive *currentDive); void tripChanged(dive_trip *trip, TripField); void filterReset(); @@ -194,6 +196,7 @@ public slots: // Does nothing in list view. //void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector &dives); void divesSelected(const QVector &dives, dive *currentDive); + void tripSelected(dive_trip *trip, dive *currentDive); void filterReset(); public: diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp index 3b2381f78..ba23588fd 100644 --- a/qt-models/filtermodels.cpp +++ b/qt-models/filtermodels.cpp @@ -27,6 +27,7 @@ void MultiFilterSortModel::resetModel(DiveTripModelBase::Layout layout) setSourceModel(model.get()); connect(model.get(), &DiveTripModelBase::selectionChanged, this, &MultiFilterSortModel::selectionChangedSlot); + connect(model.get(), &DiveTripModelBase::tripSelected, this, &MultiFilterSortModel::tripSelectedSlot); model->initSelection(); LocationInformationModel::instance()->update(); } @@ -45,6 +46,16 @@ void MultiFilterSortModel::selectionChangedSlot(const QVector &indi emit selectionChanged(indicesLocal, mapFromSource(currentDive)); } +// Translate selection into local indices and re-emit signal +void MultiFilterSortModel::tripSelectedSlot(QModelIndex trip, QModelIndex currentDive) +{ + QModelIndex local = mapFromSource(trip); + if (!local.isValid()) + return; + + emit tripSelected(local, mapFromSource(currentDive)); +} + bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const { return true; diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h index 27ff21943..a9fbd167e 100644 --- a/qt-models/filtermodels.h +++ b/qt-models/filtermodels.h @@ -19,8 +19,10 @@ public: void resetModel(DiveTripModelBase::Layout layout); signals: void selectionChanged(const QVector &indices, QModelIndex currentDive); + void tripSelected(QModelIndex trip, QModelIndex currentDive); private slots: void selectionChangedSlot(const QVector &indices, QModelIndex currentDive); + void tripSelectedSlot(QModelIndex trip, QModelIndex currentDive); private: MultiFilterSortModel(QObject *parent = 0); std::unique_ptr model;