Merge lp:~azzar1/unity/autoselect-all-button-checkoptionfilter into lp:unity

[Andrea Azzarone]

Auto select the "All" button if all checkoption buttons are selected.

There was already a test for it, so i've just updated it.

[Sam Spilsbury (smspillaz)]

review: Approve

[Michal Hruby (mhr3)]

Looks good.

review: Approve

[Andrea Azzarone (azzar1)]

Requested JonhLea's review so he can confirm the wanted behavior.

[John Lea (johnlea)]

Hi, many thanks for this fix. After testing this fix we would like to request one change to the originally requested functionality before this lands.

If a user selects all of the filters in a section, the 'All' button should be automatically selected. In this state clicking on the 'All' button should do nothing. If the user deselects one of the filters the 'All' button should be automatically be de-selected.

review: Needs Fixing (design review)

[Michal Hruby (mhr3)]

@John, could you re-consider, I think this would make behaviour of the filters quite inconsistent, consider this:

1) User selects a couple of options, and continues doing so until all of them are selected - at this point the All button is highlighted (and also all the options), user switches to a different lens looks at the filters and sees the All button highlighted but none of the options selected - inconsistency.

[John Lea (johnlea)]

mhr3; we need to balance this issue with other issues, so waiting for this branch to be available so that we can test the requested behaviour before making a final call.

=== modified file 'UnityCore/CheckOptionFilter.cpp'
--- UnityCore/CheckOptionFilter.cpp	2011-08-05 19:25:57 +0000
+++ UnityCore/CheckOptionFilter.cpp	2012-01-23 09:50:29 +0000
@@ -88,7 +88,9 @@
 {
   if (!IsValid())
     return;
-  gboolean raw_filtering = FALSE;
+    
+  bool raw_filtering = false;
+  bool all_activated = true;
 
   GVariantBuilder options;
   g_variant_builder_init(&options, G_VARIANT_TYPE("a(sssb)"));
@@ -100,13 +102,16 @@
     std::string icon_hint = option->icon_hint;
     bool active = option->active;
 
-    raw_filtering = raw_filtering ? TRUE : active;
+    raw_filtering |= active;
+    all_activated &= active;
 
     g_variant_builder_add(&options, "(sssb)",
                           id.c_str(), name.c_str(),
                           icon_hint.c_str(), active ? TRUE : FALSE);
   }
-  
+
+  raw_filtering = all_activated ? FALSE : raw_filtering;
+
   GVariantBuilder hints;
   g_variant_builder_init(&hints, G_VARIANT_TYPE("a{sv}"));
   g_variant_builder_add(&hints, "{sv}", "options", g_variant_builder_end(&options));
@@ -117,7 +122,7 @@
                       g_variant_builder_end(&hints));
   dee_model_set_value(model_, iter_,
                       FilterColumn::FILTERING,
-                      g_variant_new("b", raw_filtering));
+                      g_variant_new("b", raw_filtering ? TRUE : FALSE));
   IgnoreChanges(false);
 
   filtering.EmitChanged(filtering);

=== modified file 'tests/test_lens.cpp'
--- tests/test_lens.cpp	2011-12-02 12:03:27 +0000
+++ tests/test_lens.cpp	2012-01-23 09:50:29 +0000
@@ -394,7 +394,7 @@
   EXPECT_FALSE (options[2]->active);
 
   options[2]->active = true;
-  EXPECT_TRUE (filter->filtering);
+  EXPECT_FALSE (filter->filtering);
   EXPECT_TRUE (options[0]->active);
   EXPECT_TRUE (options[1]->active);
   EXPECT_TRUE (options[2]->active);