Merge lp:~brandontschaefer/unity/ql-right-click-fixes into lp:unity

[Brandon Schaefer]

Right clicking a launcher icon will open its quicklist even if a quicklist is already open.

=== Problem ===
If you right click a launcher icon, then right click another launcher icon it will not open that quicklist.

=== Fix ===
Instead of opening the quicklist on mouse pressed, open it on mouse released. So when a click outside of the quicklist happens that event can activate the quicklist.

=== Test ===
AP test

[Marco Trevisan (Treviño) (3v1n0)]

After fixing the multimonitor issue, +1 from me ;)

review: Approve

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/983/console reported an error when processing this lp:~brandontschaefer/unity/ql-right-click-fixes branch.
Not merging it.

[Marco Trevisan (Treviño) (3v1n0)]

Oh, noticed that clicking again on a ql causes it to be closed and re-opened. It should stay.

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

Another thing... I think we shouldn't change the behavior of the click, so the quicklist should be always be opened on mouse-down, not on mouse-up.

At this point you should probably refactor the quicklist and the manager, so that QuicklistView::RecvMouseDownOutsideOfQuicklist won't always hide a quicklist, but we'll hide it only if the clicked area there's not a "quicklist owner".

[Marco Trevisan (Treviño) (3v1n0)]

This solution is better, just one thing:

Use OpenQuicklist(false, monitor); or it won't work in multi-monitor setup.

The _open_quicklist is a little bit confusing, since it seems to indicate that the quicklist is currently open, while it 's actually used allow a quicklist to open... Probably inverting the logic could be better.

72 - mouse_down_outside_pointer_grab_area.connect(sigc::mem_fun(this, &QuicklistView::RecvMouseDownOutsideOfQuicklist));

Mhmh... Maybe it's better not to remove this as well, I'd just Hide the quicklist in QuicklistView, and I'd connect to the same signal on LauncherIcon to set the value you need...

[Marco Trevisan (Treviño) (3v1n0)]

Nice, approved!

review: Approve

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1018/console reported an error when processing this lp:~brandontschaefer/unity/ql-right-click-fixes branch.
Not merging it.

[Unity Merger (unity-merger)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve (quality)

=== modified file 'launcher/LauncherIcon.cpp'
--- launcher/LauncherIcon.cpp	2012-08-02 11:30:02 +0000
+++ launcher/LauncherIcon.cpp	2012-08-13 17:23:38 +0000
@@ -87,6 +87,7 @@
   , _last_stable(max_num_monitors)
   , _parent_geo(max_num_monitors)
   , _saved_center(max_num_monitors)
+  , _allow_quicklist_to_show(true)
 {
   for (unsigned i = 0; i < unsigned(Quirk::LAST); i++)
   {
@@ -153,6 +154,11 @@
   _quicklist = new QuicklistView();
   AddChild(_quicklist.GetPointer());
 
+  _quicklist->mouse_down_outside_pointer_grab_area.connect([&] (int x, int y, unsigned long button_flags, unsigned long key_flags)
+  {
+    _allow_quicklist_to_show = false;
+  });
+
   QuicklistManager::Default()->RegisterQuicklist(_quicklist.GetPointer());
 }
 
@@ -532,6 +538,7 @@
 void LauncherIcon::RecvMouseLeave(int monitor)
 {
   _last_monitor = -1;
+  _allow_quicklist_to_show = true;
 
   if (_tooltip)
     _tooltip->ShowWindow(false);
@@ -624,16 +631,24 @@
 void LauncherIcon::RecvMouseDown(int button, int monitor, unsigned long key_flags)
 {
   if (button == 3)
-    OpenQuicklist();
+    OpenQuicklist(false, monitor);
 }
 
 void LauncherIcon::RecvMouseUp(int button, int monitor, unsigned long key_flags)
 {
   if (button == 3)
   {
+    if (_allow_quicklist_to_show)
+    {
+      OpenQuicklist(false, monitor);
+    }
+
     if (_quicklist && _quicklist->IsVisible())
+    {
       _quicklist->CaptureMouseDownAnyWhereElse(true);
+    }
   }
+  _allow_quicklist_to_show = true;
 }
 
 void LauncherIcon::RecvMouseClick(int button, int monitor, unsigned long key_flags)

=== modified file 'launcher/LauncherIcon.h'
--- launcher/LauncherIcon.h	2012-08-02 11:30:02 +0000
+++ launcher/LauncherIcon.h	2012-08-13 17:23:38 +0000
@@ -339,6 +339,8 @@
   bool             _quirks[unsigned(Quirk::LAST)];
   struct timespec  _quirk_times[unsigned(Quirk::LAST)];
 
+  bool             _allow_quicklist_to_show;
+
   std::list<LauncherEntryRemote::Ptr> _entry_list;
 
 protected:

=== modified file 'tests/autopilot/unity/tests/test_quicklist.py'
--- tests/autopilot/unity/tests/test_quicklist.py	2012-08-09 01:37:02 +0000
+++ tests/autopilot/unity/tests/test_quicklist.py	2012-08-13 17:23:38 +0000
@@ -155,6 +155,44 @@
         self.addCleanup(self.dash.ensure_hidden)
         self.assertThat(self.dash.visible, Eventually(Equals(True)))
 
+    def test_right_click_opens_quicklist_if_already_open(self):
+        """A right click to another icon in the launcher must
+        close the current open quicklist and open the other
+        icons quicklist.
+        lp:890991
+        """
+
+        calc_win = self.start_app_window("Calculator")
+        mahj_win = self.start_app_window("Mahjongg")
+
+        calc_icon = self.launcher.model.get_icon(
+            desktop_id=calc_win.application.desktop_file)
+        mahj_icon = self.launcher.model.get_icon(
+            desktop_id=mahj_win.application.desktop_file)
+
+        calc_ql = self.open_quicklist_for_icon(calc_icon)
+        self.assertThat(calc_ql.active, Eventually(Equals(True)))
+
+        mahj_ql = self.open_quicklist_for_icon(mahj_icon)
+        self.assertThat(mahj_ql.active, Eventually(Equals(True)))
+        self.assertThat(calc_ql.active, Eventually(Equals(False)))
+
+    def test_right_clicking_same_icon_doesnt_reopen_ql(self):
+        """A right click to the same icon in the launcher must
+        not re-open the quicklist if already open. It must hide.
+        """
+
+        calc_win = self.start_app_window("Calculator")
+
+        calc_icon = self.launcher.model.get_icon(
+            desktop_id=calc_win.application.desktop_file)
+
+        calc_ql = self.open_quicklist_for_icon(calc_icon)
+        self.assertThat(calc_ql.active, Eventually(Equals(True)))
+
+        calc_ql = self.open_quicklist_for_icon(calc_icon)
+        self.assertThat(calc_ql.active, Eventually(Equals(False)))
+
 
 class QuicklistKeyNavigationTests(UnityTestCase):
     """Tests for the quicklist key navigation."""