From bc1ff9a1213e4c2a0c135974cbce03bc7614d7b5 Mon Sep 17 00:00:00 2001 From: Dirk Hohndel Date: Fri, 17 Aug 2012 14:23:38 -0700 Subject: [PATCH 1/2] More fiddling with the selection As expected, this is pretty subtle to get right. But with this change the code becomes simpler and more straight forward, I think. If the dives in a group are collapsed, we don't even try to make gtk keep track of their selection status - we explicitly do so ourselves. This avoids the artificial expand / collapse around our attempt to force gtk to allow us to select children that are hidden. But if a dive is expanded, then we trust gtk to get things right. Signed-off-by: Dirk Hohndel --- divelist.c | 52 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/divelist.c b/divelist.c index 0e1b40dd6..b1b74d45a 100644 --- a/divelist.c +++ b/divelist.c @@ -169,7 +169,6 @@ static void first_leaf(GtkTreeModel *model, GtkTreeIter *iter, int *diveidx) if(!gtk_tree_view_row_expanded(GTK_TREE_VIEW(dive_list.tree_view), tpath)) gtk_tree_view_expand_row(GTK_TREE_VIEW(dive_list.tree_view), tpath, FALSE); gtk_tree_model_get(GTK_TREE_MODEL(model), iter, DIVE_INDEX, diveidx, -1); - track_select(*diveidx); } } @@ -179,31 +178,36 @@ static void select_children(GtkTreeModel *model, GtkTreeSelection * selection, GtkTreeIter *iter, gboolean was_selected) { int i, nr_children; - gboolean unexpand = FALSE; + gboolean expanded = FALSE; GtkTreeIter parent; GtkTreePath *tpath; memcpy(&parent, iter, sizeof(parent)); tpath = gtk_tree_model_get_path(model, &parent); - - /* stupid gtk doesn't allow us to select rows that are invisible; so if the - user clicks on a row that isn't expanded, we briefly expand it, select the - children, and then unexpand it again */ - if(!gtk_tree_view_row_expanded(GTK_TREE_VIEW(dive_list.tree_view), tpath)) { - unexpand = TRUE; - gtk_tree_view_expand_row(GTK_TREE_VIEW(dive_list.tree_view), tpath, FALSE); - } + expanded = gtk_tree_view_row_expanded(GTK_TREE_VIEW(dive_list.tree_view), tpath); nr_children = gtk_tree_model_iter_n_children(model, &parent); for (i = 0; i < nr_children; i++) { gtk_tree_model_iter_nth_child(model, iter, &parent, i); - if (was_selected) - gtk_tree_selection_unselect_iter(selection, iter); - else - gtk_tree_selection_select_iter(selection, iter); + + /* if the parent is expanded, just (un)select the children and we'll + track their selection status in the callback + otherwise just change the selection status directly without + bothering gtk */ + if (expanded) { + if (was_selected) + gtk_tree_selection_unselect_iter(selection, iter); + else + gtk_tree_selection_select_iter(selection, iter); + } else { + int idx; + gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, -1); + if (was_selected) + track_unselect(idx); + else + track_select(idx); + } } - if (unexpand) - gtk_tree_view_collapse_row(GTK_TREE_VIEW(dive_list.tree_view), tpath); } /* make sure that if we expand a summary row that is selected, the children show @@ -228,11 +232,17 @@ gboolean modify_selection_cb(GtkTreeSelection *selection, GtkTreeModel *model, if (gtk_tree_model_get_iter(model, &iter, path)) { gtk_tree_model_get(model, &iter, DIVE_INDEX, &dive_idx, -1); /* turns out we need to move the selectiontracker here */ - if (was_selected) - track_unselect(dive_idx); - else - track_select(dive_idx); - if (dive_idx < 0) { + +#if DEBUG_SELECTION_TRACKING + printf("modify_selection_cb with idx %d (according to gtk was %sselected) - ", + dive_idx, was_selected ? "" : "un"); +#endif + if (dive_idx >= 0) { + if (was_selected) + track_unselect(dive_idx); + else + track_select(dive_idx); + } else { select_children(model, selection, &iter, was_selected); } } From 9e9ba73b3d21c2184c16f3ba5cf3a71f7c662a55 Mon Sep 17 00:00:00 2001 From: Dirk Hohndel Date: Fri, 17 Aug 2012 15:03:57 -0700 Subject: [PATCH 2/2] Another selection fix The corner cases are getting more and more artificial. Without this patch, the following can happen: Select one or more dives in an (expanded) dive trip. Now collapse that trip with the little triangle. Select a different trip. The previously selected dive(s) are still part of the selection (as you can see, for example, in the statistics tab). With this patch the scenario above works as intended (all the dives in the new trip are selected), but we have another corner case: Just as before, select one or more dives in an expanded dive trip. Collapse that trip and ctrl-click on another trip. Now you lose the originally selected dives. Frankly, if you ctrl-click to add more dives to your selection - just don't collapse the trips the dives are in? As this new corner case seems even more artificial than the previous one, I consider this patch an improvement. But fundamentally I am just battling all the ways in which gtk's selection handling is messed up. When I get the selection call back I cannot tell if this is a new selection or an incremental selection (i.e., a shift-click or ctrl-click). Signed-off-by: Dirk Hohndel --- divelist.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/divelist.c b/divelist.c index b1b74d45a..0f5c90002 100644 --- a/divelist.c +++ b/divelist.c @@ -152,6 +152,11 @@ void track_unselect(int idx) #endif } +void clear_tracker(void) +{ + amount_selected = 0; +} + /* when subsurface starts we want to have the last dive selected. So we simply walk to the first leaf (and skip the summary entries - which have negative DIVE_INDEX) */ @@ -229,12 +234,21 @@ gboolean modify_selection_cb(GtkTreeSelection *selection, GtkTreeModel *model, GtkTreeIter iter; int dive_idx; + /* if gtk thinks nothing is selected we should clear out our + tracker as well - otherwise hidden selected rows can stay + "stuck". The down side is that we now have a different bug: + If you select a dive, collapse the dive trip and ctrl-click + another dive trip, the initial dive is no longer selected. + Just don't do that, ok? */ + if (gtk_tree_selection_count_selected_rows(selection) == 0) + clear_tracker(); + if (gtk_tree_model_get_iter(model, &iter, path)) { gtk_tree_model_get(model, &iter, DIVE_INDEX, &dive_idx, -1); /* turns out we need to move the selectiontracker here */ #if DEBUG_SELECTION_TRACKING - printf("modify_selection_cb with idx %d (according to gtk was %sselected) - ", + printf("modify_selection_cb with idx %d (according to gtk was %sselected)\n", dive_idx, was_selected ? "" : "un"); #endif if (dive_idx >= 0) {