Fix selection logic when manually adding a dive

This was an interesting bug. When adding a dive that would end up in the
middle of the dive list, the newest dive in the dive list would end up
marked in the dive structure as selected - even though it wasn't
visualized as selected by Qt. Bad things happen if the user then made
changes to that dive without selecting something else first, for example
by either editing the dive or doing things to it like removing it from or
adding it to a trip. The same operation would also be applied to the
newest dive in the dive list.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

2 files changed, 14 insertions, 5 deletions

diff --git a/qt-ui/divelistview.cpp b/qt-ui/divelistview.cpp
index 117d02247..0d0d999f4 100644
--- a/qt-ui/divelistview.cpp
+++ b/qt-ui/divelistview.cpp
@@ -189,13 +189,19 @@ void DiveListView::selectTrip(dive_trip_t *trip)
 
 void DiveListView::unselectDives()
 {
+	// make sure we don't try to redraw the dives during the selection change
+	selected_dive = -1;
+	amount_selected = 0;
+	// clear the Qt selection
 	selectionModel()->clearSelection();
 	// clearSelection should emit selectionChanged() but sometimes that
 	// appears not to happen
+	// since we are unselecting all dives there is no need to use deselect_dive() - that
+	// would only cause pointless churn
 	int i;
 	struct dive *dive;
 	for_each_dive(i, dive) {
-		deselect_dive(i);
+		dive->selected = false;
 	}
 }
 
diff --git a/qt-ui/maintab.cpp b/qt-ui/maintab.cpp
index 6bb6f14f8..f381d72d5 100644
--- a/qt-ui/maintab.cpp
+++ b/qt-ui/maintab.cpp
@@ -671,7 +671,10 @@ void MainTab::acceptChanges()
 		struct dive *added_dive = clone_dive(&displayed_dive);
 		record_dive(added_dive);
 		addedId = added_dive->id;
-		// unselect everything as far as the UI is concerned - we'll fix that below
+		// unselect everything as far as the UI is concerned and select the new
+		// dive - we'll have to undo/redo this later after we resort the dive_table
+		// but we need the dive selected for the middle part of this function - this
+		// way we can reuse the code used for editing dives
 		MainWindow::instance()->dive_list()->unselectDives();
 		selected_dive = get_divenr(added_dive);
 		amount_selected = 1;
@@ -779,12 +782,12 @@ void MainTab::acceptChanges()
 	int scrolledBy = MainWindow::instance()->dive_list()->verticalScrollBar()->sliderPosition();
 	resetPallete();
 	if (editMode == ADD || editMode == MANUALLY_ADDED_DIVE) {
-		// now let's resort the dive list and make sure the newly added dive is still selected
-		selected_dive = -1;
-		amount_selected = 0;
+		// since a newly added dive could be in the middle of the dive_table we need
+		// to resort the dive list and make sure the newly added dive gets selected again
 		sort_table(&dive_table);
 		MainWindow::instance()->dive_list()->reload(DiveTripModel::CURRENT, true);
 		int newDiveNr = get_divenr(get_dive_by_uniq_id(addedId));
+		MainWindow::instance()->dive_list()->unselectDives();
 		MainWindow::instance()->dive_list()->selectDive(newDiveNr, true);
 		editMode = NONE;
 		MainWindow::instance()->refreshDisplay();