mifr/subsurface
1
0
Fork 0
mirror of https://github.com/subsurface/subsurface.git synced 2025-01-19 06:15:26 +00:00
Code Issues Projects Releases Packages Wiki Activity

Undo: unify selection behavior in dive-list commands

Browse source 
Some commands tried to retain the current selection on undo/redo,
others set the selection to the modified dives.

The latter was introduced because it was easier in some cases, but
it is probably more user-friendly because the user gets feedback
on the change.

Therefore, unify to always select the affected dives on undo()/redo().

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2019-06-23 11:13:41 +02:00 • committed by bstoeger
parent 27944a52b1
commit e1abf9485c
7 changed files with 48 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
core/subsurface-qt/DiveListNotifier.h
View file

@ -53,13 +53,12 @@ signals:
void tripChanged(dive_trip *trip, TripField field);
// Selection-signals come in two kinds:
// - divesSelected, divesDeselected and currentDiveChanged are are used by the dive-list
// - divesSelected and currentDiveChanged are are used by the dive-list
// model and view to correctly highlight the correct dives.
// - selectionChanged() is called once at the end of commands if either the selection
// or the current dive changed. It is used by the main-window / profile to update
// their data.
void divesSelected(const QVector<dive *> &dives);
void divesDeselected(const QVector<dive *> &dives);
void currentDiveChanged();
void selectionChanged();

17
desktop-widgets/command_divelist.cpp
View file

@ -614,6 +614,9 @@ void ShiftTime::redoit()
emit diveListNotifier.divesTimeChanged(timeChanged, diveList);
emit diveListNotifier.divesChanged(diveList, DiveField::DATETIME);
// Select the changed dives
restoreSelection(diveList.toStdVector(), diveList[0]);
// Negate the time-shift so that the next call does the reverse
timeChanged = -timeChanged;
}
@ -638,6 +641,13 @@ RenumberDives::RenumberDives(const QVector<QPair<dive *, int>> &divesToRenumberI
void RenumberDives::undoit()
{
renumberDives(divesToRenumber);
// Select the changed dives
std::vector<dive *> dives;
dives.reserve(divesToRenumber.size());
for (const QPair<dive *, int> &item: divesToRenumber)
dives.push_back(item.first);
restoreSelection(dives, dives[0]);
}
bool RenumberDives::workToBeDone()
@ -660,6 +670,13 @@ void TripBase::redoit()
{
moveDivesBetweenTrips(divesToMove);
sort_trip_table(&trip_table); // Though unlikely, moving dives may reorder trips
// Select the moved dives
std::vector<dive *> dives;
dives.reserve(divesToMove.divesToMove.size());
for (const DiveToTrip &item: divesToMove.divesToMove)
dives.push_back(item.dive);
restoreSelection(dives, dives[0]);
}
void TripBase::undoit()

20
desktop-widgets/command_private.cpp
View file

@ -47,12 +47,13 @@ bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
// To do so, generate vectors of dives to be selected and deselected.
// We send signals batched by trip, so keep track of trip/dive pairs.
QVector<dive *> divesToSelect;
QVector<dive *> divesToDeselect;
divesToSelect.reserve(selection.size());
// TODO: We might want to keep track of selected dives in a more efficient way!
int i;
dive *d;
amount_selected = 0; // We recalculate amount_selected
bool selectionChanged = false;
for_each_dive(i, d) {
// We only modify dives that are currently visible.
if (d->hidden_by_filter) {
@ -65,24 +66,21 @@ bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
// TODO: By sorting the list in the same way as the backend, this could be made more efficient.
bool newState = std::find(selection.begin(), selection.end(), d) != selection.end();
if (newState) {
++amount_selected;
divesToSelect.push_back(d);
}
// TODO: Instead of using select_dive() and deselect_dive(), we set selected directly.
// The reason is that deselect() automatically sets a new current dive, which we
// don't want, as we set it later anyway.
// There is other parts of the C++ code that touches the innards directly, but
// ultimately this should be pushed down to C.
if (newState && !d->selected) {
d->selected = true;
++amount_selected;
divesToSelect.push_back(d);
} else if (!newState && d->selected) {
d->selected = false;
divesToDeselect.push_back(d);
}
selectionChanged |= d->selected != newState;
d->selected = newState;
}
// Send the select and deselect signals
emit diveListNotifier.divesSelected(divesToSelect);
emit diveListNotifier.divesDeselected(divesToDeselect);
bool currentDiveChanged = false;
if (!currentDive) {
@ -105,7 +103,7 @@ bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
}
// return true if selection of current dive changed
return !divesToSelect.empty() || !divesToDeselect.empty() || currentDiveChanged;
return selectionChanged || currentDiveChanged;
}
// Turn current selection into a vector.

10
desktop-widgets/divelistview.cpp
View file

@ -192,13 +192,11 @@ void DiveListView::reset()
}
// If items were selected, inform the selection model
void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes, bool select)
void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes)
{
clearSelection();
MultiFilterSortModel *m = MultiFilterSortModel::instance();
QItemSelectionModel *s = selectionModel();
auto flags = select ?
QItemSelectionModel::Rows | QItemSelectionModel::Select :
QItemSelectionModel::Rows | QItemSelectionModel::Deselect;
for (const QModelIndex &index: indexes) {
// We have to transform the indices into local indices, since
// there might be sorting or filtering in effect.
@ -210,10 +208,10 @@ void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes, boo
if (!localIndex.isValid())
continue;
s->select(localIndex, flags);
s->select(localIndex, QItemSelectionModel::Rows | QItemSelectionModel::Select);
// If an item of a not-yet expanded trip is selected, expand the trip.
if (select && localIndex.parent().isValid() && !isExpanded(localIndex.parent())) {
if (localIndex.parent().isValid() && !isExpanded(localIndex.parent())) {
setAnimated(false);
expand(localIndex.parent());
setAnimated(true);

2
desktop-widgets/divelistview.h
View file

@ -63,7 +63,7 @@ slots:
void shiftTimes();
void loadImages();
void loadWebImages();
void diveSelectionChanged(const QVector<QModelIndex> &indexes, bool select);
void diveSelectionChanged(const QVector<QModelIndex> &indexes);
void currentDiveChanged(QModelIndex index);
void filterFinished();
void tripChanged(dive_trip *trip, TripField);

38
qt-models/divetripmodel.cpp
View file

@ -417,16 +417,6 @@ bool DiveTripModelBase::setData(const QModelIndex &index, const QVariant &value,
return true;
}
void DiveTripModelBase::divesSelected(const QVector<dive *> &dives)
{
changeDiveSelection(dives, true);
}
void DiveTripModelBase::divesDeselected(const QVector<dive *> &dives)
{
changeDiveSelection(dives, false);
}
// Find a range of matching elements in a vector.
// Input parameters:
// v: vector to be searched
@ -561,7 +551,6 @@ 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::divesDeselected, this, &DiveTripModelTree::divesDeselected);
connect(&diveListNotifier, &DiveListNotifier::currentDiveChanged, this, &DiveTripModelTree::currentDiveChanged);
connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &DiveTripModelTree::tripChanged);
@ -1037,7 +1026,6 @@ void DiveTripModelTree::divesMovedBetweenTrips(dive_trip *from, dive_trip *to, b
QVector<dive *> selectedDives = filterSelectedDives(dives);
divesAdded(to, createTo, dives);
divesDeleted(from, deleteFrom, dives);
changeDiveSelectionTrip(to, dives, true);
}
void DiveTripModelTree::divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives)
@ -1061,22 +1049,23 @@ void DiveTripModelTree::divesTimeChangedTrip(dive_trip *trip, timestamp_t delta,
QVector<dive *> selectedDives = filterSelectedDives(dives);
divesDeleted(trip, false, dives);
divesAdded(trip, false, dives);
changeDiveSelectionTrip(trip, selectedDives, true);
}
void DiveTripModelTree::changeDiveSelection(const QVector<dive *> &dives, bool select)
{
processByTrip(dives, [this, select] (dive_trip *trip, const QVector<dive *> &divesInTrip)
{ changeDiveSelectionTrip(trip, divesInTrip, select); });
}
void DiveTripModelTree::changeDiveSelectionTrip(dive_trip *trip, const QVector<dive *> &dives, bool select)
void DiveTripModelTree::divesSelected(const QVector<dive *> &dives)
{
// We got a number of dives that have been selected. Turn this into QModelIndexes and
// emit a signal, so that views can change the selection.
QVector<QModelIndex> indexes;
indexes.reserve(dives.count());
processByTrip(dives, [this, &indexes] (dive_trip *trip, const QVector<dive *> &divesInTrip)
{ divesSelectedTrip(trip, divesInTrip, indexes); });
emit selectionChanged(indexes);
}
void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector<dive *> &dives, QVector<QModelIndex> &indexes)
{
if (!trip) {
// This is at the top level.
// Since both lists are sorted, we can do this linearly. Perhaps a binary search
@ -1095,7 +1084,7 @@ void DiveTripModelTree::changeDiveSelectionTrip(dive_trip *trip, const QVector<d
if (idx < 0) {
// We don't know the trip - this shouldn't happen. We seem to have
// missed some signals!
qWarning() << "DiveTripModelTree::changeDiveSelection(): unknown trip";
qWarning() << "DiveTripModelTree::diveSelectedTrip(): unknown trip";
return;
}
// Locate the indices inside the trip.
@ -1111,8 +1100,6 @@ void DiveTripModelTree::changeDiveSelectionTrip(dive_trip *trip, const QVector<d
indexes.append(createIndex(j, 0, idx));
}
}
emit selectionChanged(indexes, select);
}
void DiveTripModelTree::currentDiveChanged()
@ -1182,7 +1169,6 @@ 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::divesDeselected, this, &DiveTripModelList::divesDeselected);
connect(&diveListNotifier, &DiveListNotifier::currentDiveChanged, this, &DiveTripModelList::currentDiveChanged);
// Fill model
@ -1302,7 +1288,7 @@ void DiveTripModelList::divesTimeChanged(timestamp_t delta, const QVector<dive *
divesSelected(selectedDives);
}
void DiveTripModelList::changeDiveSelection(const QVector<dive *> &dives, bool select)
void DiveTripModelList::divesSelected(const QVector<dive *> &dives)
{
// We got a number of dives that have been selected. Turn this into QModelIndexes and
// emit a signal, so that views can change the selection.
@ -1320,7 +1306,7 @@ void DiveTripModelList::changeDiveSelection(const QVector<dive *> &dives, bool s
indexes.append(createIndex(j, 0, noParent));
}
emit selectionChanged(indexes, select);
emit selectionChanged(indexes);
}
void DiveTripModelList::currentDiveChanged()

14
qt-models/divetripmodel.h
View file

@ -89,19 +89,13 @@ signals:
// into QModelIndexes according to the current view (tree/list). Finally, the DiveListView transforms these
// indexes into local indexes according to current sorting/filtering and instructs the QSelectionModel to
// perform the appropriate actions.
void selectionChanged(const QVector<QModelIndex> &indexes, bool select);
void selectionChanged(const QVector<QModelIndex> &indexes);
void newCurrentDive(QModelIndex index);
protected slots:
void divesSelected(const QVector<dive *> &dives);
void divesDeselected(const QVector<dive *> &dives);
protected:
// Access trip and dive data
static QVariant diveData(const struct dive *d, int column, int role);
static QVariant tripData(const dive_trip *trip, int column, int role);
// Select or deselect dives
virtual void changeDiveSelection(const QVector<dive *> &dives, bool select) = 0;
virtual dive *diveOrNull(const QModelIndex &index) const = 0; // Returns a dive if this index represents a dive, null otherwise
};
@ -114,6 +108,7 @@ public slots:
void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives);
void divesChanged(const QVector<dive *> &dives);
void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
void divesSelected(const QVector<dive *> &dives);
void currentDiveChanged();
void tripChanged(dive_trip *trip, TripField);
@ -126,8 +121,7 @@ private:
QVariant data(const QModelIndex &index, int role) const override;
void filterFinished() override;
bool lessThan(const QModelIndex &i1, const QModelIndex &i2) const override;
void changeDiveSelection(const QVector<dive *> &dives, bool select) override;
void changeDiveSelectionTrip(dive_trip *trip, const QVector<dive *> &dives, bool select);
void divesSelectedTrip(dive_trip *trip, const QVector<dive *> &dives, QVector<QModelIndex> &);
dive *diveOrNull(const QModelIndex &index) const override;
bool setShown(const QModelIndex &idx, bool shown);
void divesChangedTrip(dive_trip *trip, const QVector<dive *> &dives);
@ -181,6 +175,7 @@ public slots:
void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
// Does nothing in list view.
//void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives);
void divesSelected(const QVector<dive *> &dives);
void currentDiveChanged();
public:
@ -192,7 +187,6 @@ private:
QVariant data(const QModelIndex &index, int role) const override;
void filterFinished() override;
bool lessThan(const QModelIndex &i1, const QModelIndex &i2) const override;
void changeDiveSelection(const QVector<dive *> &dives, bool select) override;
dive *diveOrNull(const QModelIndex &index) const override;
bool setShown(const QModelIndex &idx, bool shown);