Merge lp:~brandontschaefer/unity/lp.1215630 into lp:unity

[Brandon Schaefer]

Only change the selection of the icon if the delta x/y is greater then 3 (either direction).

Only change the selection of the icon if the delta x/y is greater then 3 (either direction).

This does mean you could slowly move across an icon without it changing.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3478
http://jenkins.qa.ubuntu.com/job/unity-ci/309/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/198
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/196
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/197

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/309/rebuild

review: Approve (continuous-integration)

[Christopher Townsend (townsend)]

Works as intended, but a couple of things.

- I'm wondering why you switched the const and var type in the constant variable declarations. Seems easier to read with const placed first.
- Also, is it possible to have an AP test for testing a slight mouse bump (not sure if it is)?

review: Needs Information

[lp:~brandontschaefer/unity/lp.1215630]

3479. By Brandon Schaefer on 2013-08-30

* On the first time the switcher view is opened, check if the mouse is over an icon.
  if so ignore it until it enters a new icon.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3479
http://jenkins.qa.ubuntu.com/job/unity-ci/326/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/215
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/213/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/214

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/326/rebuild

review: Needs Fixing (continuous-integration)

[lp:~brandontschaefer/unity/lp.1215630]

3480. By Brandon Schaefer on 2013-08-31

* Clean up switcher mouse ap test
* Add new AP test

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3480
http://jenkins.qa.ubuntu.com/job/unity-ci/327/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/216
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/214
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/215

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/327/rebuild

review: Approve (continuous-integration)

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

The approach seems fine to me, but this would lead to checking the mouse pointer on start whether there's a mouse movement or not, and it does that in sync way (blame Xorg!)... So why not just using something like this instead: http://pastebin.ubuntu.com/6055505/ ?

[Brandon Schaefer (brandontschaefer)]

Thats a way better solution... Ill get to this tomorrow (as I shouldn't be checking my email ;).

[Brandon Schaefer (brandontschaefer)]

REJECTED!!! As there is a new branch here:
https://code.launchpad.net/~brandontschaefer/unity/lp-1215630-fix-v2/+merge/184144

Unmerged revisions

3480. By Brandon Schaefer on 2013-08-31

* Clean up switcher mouse ap test
* Add new AP test

3479. By Brandon Schaefer on 2013-08-30

* On the first time the switcher view is opened, check if the mouse is over an icon.
  if so ignore it until it enters a new icon.

3478. By Brandon Schaefer on 2013-08-23

* If the mouse move delta x/y is to small, ignore that event. Dont
  want accidental bumping to cause unwanted selection changes.

=== modified file 'launcher/SwitcherView.cpp'
--- launcher/SwitcherView.cpp	2013-08-09 17:51:06 +0000
+++ launcher/SwitcherView.cpp	2013-08-31 01:29:50 +0000
@@ -36,9 +36,9 @@
 
 namespace
 {
-  const unsigned int VERTICAL_PADDING = 45;
-  const unsigned int SPREAD_OFFSET = 100;
-  const unsigned int EXTRA_ICON_SPACE = 10;
+  unsigned int const VERTICAL_PADDING = 45;
+  unsigned int const SPREAD_OFFSET = 100;
+  unsigned int const EXTRA_ICON_SPACE = 10;
 }
 
 NUX_IMPLEMENT_OBJECT_TYPE(SwitcherView);
@@ -61,6 +61,7 @@
   , last_icon_selected_(-1)
   , last_detail_icon_selected_(-1)
   , target_sizes_set_(false)
+  , check_mouse_first_time_(true)
 {
   icon_renderer_->pip_style = OVER_TILE;
   icon_renderer_->monitor = monitors::MAX;
@@ -114,6 +115,7 @@
   .add("animation-length", animation_length)
   .add("spread-size", (float)spread_size)
   .add("label", text_view_->GetText())
+  .add("last_icon_selected", last_icon_selected_)
   .add("spread_offset", SPREAD_OFFSET)
   .add("label_visible", text_view_->IsVisible());
 }
@@ -232,7 +234,7 @@
   return {geo.x + x, geo.y + y};
 }
 
-void SwitcherView::RecvMouseMove(int x, int y, int /*dx*/, int /*dy*/, unsigned long /*button_flags*/, unsigned long /*key_flags*/)
+void SwitcherView::RecvMouseMove(int x, int y, int dx, int dy, unsigned long /*button_flags*/, unsigned long /*key_flags*/)
 {
   if (model_->detail_selection)
   {
@@ -773,6 +775,23 @@
   last_background_ = background_geo;
 
   icon_renderer_->PreprocessIcons(last_args_, GetGeometry());
+
+  if (check_mouse_first_time_)
+  {
+    CheckMouseInsideIconOnStart();
+  }
+}
+
+void SwitcherView::CheckMouseInsideIconOnStart()
+{
+  int monitor = unity::UScreen::GetDefault()->GetMonitorWithMouse();
+  nux::Geometry const& geo = unity::UScreen::GetDefault()->GetMonitorGeometry(monitor);
+
+  nux::Point const& screen_pt = nux::GetGraphicsDisplay()->GetMouseScreenCoord();
+  nux::Point const& pt = {screen_pt.x - geo.x, screen_pt.y - geo.y};
+
+  last_icon_selected_ = IconIndexAt(pt.x - SPREAD_OFFSET, pt.y - SPREAD_OFFSET);
+  check_mouse_first_time_ = false;
 }
 
 nux::Geometry SwitcherView::GetBackgroundGeometry()

=== modified file 'launcher/SwitcherView.h'
--- launcher/SwitcherView.h	2013-08-09 17:51:06 +0000
+++ launcher/SwitcherView.h	2013-08-31 01:29:50 +0000
@@ -141,6 +141,7 @@
   void SaveLast();
 
   bool CheckMouseInsideBackground(int x, int y) const;
+  void CheckMouseInsideIconOnStart();
 
   SwitcherModel::Ptr model_;
   ui::LayoutSystem layout_system_;
@@ -150,6 +151,7 @@
   int last_icon_selected_;
   int last_detail_icon_selected_;
   bool target_sizes_set_;
+  bool check_mouse_first_time_;
 
   std::list<ui::RenderArg> last_args_;
   std::list<ui::RenderArg> saved_args_;

=== modified file 'tests/autopilot/unity/emulators/switcher.py'
--- tests/autopilot/unity/emulators/switcher.py	2013-07-17 21:54:34 +0000
+++ tests/autopilot/unity/emulators/switcher.py	2013-08-31 01:29:50 +0000
@@ -234,6 +234,10 @@
 class SwitcherView(UnityIntrospectionObject):
     """An emulator class for interacting with with SwitcherView."""
 
+    def __init__(self, *args, **kwargs):
+        super(SwitcherView, self).__init__(*args, **kwargs)
+        self._mouse = Mouse.create()
+
     @property
     def icon_args(self):
         return self.get_children_by_type(RenderArgs);
@@ -242,6 +246,21 @@
     def detail_icons(self):
         return self.get_children_by_type(LayoutWindow);
 
+    def move_over_icon(self, index):
+        offset = self.spread_offset
+        icon_arg = self.icon_args[index]
+
+        x = icon_arg.logical_center_x + offset
+        y = icon_arg.logical_center_y + offset
+
+        self._mouse.move(x,y)
+
+    def move_over_detail_icon(self, index):
+        offset = self.spread_offset
+        x = self.detail_icons[index].x + offset
+        y = self.detail_icons[index].y + offset
+
+        self._mouse.move(x,y)
 
 class RenderArgs(UnityIntrospectionObject):
   """An emulator class for interacting with the RenderArgs class."""

=== modified file 'tests/autopilot/unity/tests/test_switcher.py'
--- tests/autopilot/unity/tests/test_switcher.py	2013-08-27 21:43:12 +0000
+++ tests/autopilot/unity/tests/test_switcher.py	2013-08-31 01:29:50 +0000
@@ -576,16 +576,49 @@
         self.addCleanup(self.unity.switcher.terminate)
 
         index = self.unity.switcher.selection_index
-        offset = self.unity.switcher.view.spread_offset
-
-        icon_arg = self.unity.switcher.view.icon_args[index]
-        x = icon_arg.logical_center_x + offset
-        y = icon_arg.logical_center_y + offset
-        self.mouse.move(x, y)
+        self.unity.switcher.view.move_over_icon(index);
         self.mouse.click()
 
         self.assertProperty(char_win1, is_focused=True)
 
+    def test_mouse_doesnt_hightlight_icon_if_over_on_start(self):
+        """
+        First start the launcher and move the mosue over position of Text Editor icon,
+        then close the switcher and open it again while moving the mouse a bit.
+        Asserting that the icon does lose focus from Character Map.
+        """
+
+        char_win1, char_win2 = self.start_applications("Character Map", "Text Editor")
+        self.assertVisibleWindowStack([char_win2, char_win1])
+        self.assertProperty(char_win1, is_focused=False)
+
+        self.unity.switcher.initiate()
+        self.addCleanup(self.unity.switcher.terminate)
+
+        mouse_index = self.unity.switcher.selection_index - 1
+
+        self.unity.switcher.view.move_over_icon(mouse_index);
+        # Assert we are over the icon we want to hover over.
+        self.assertThat(self.unity.switcher.view.last_icon_selected, Eventually(Equals(mouse_index)))
+
+        self.addCleanup(self.keybinding, "switcher/cancel")
+
+        self.unity.switcher.terminate()
+        self.unity.switcher.initiate()
+
+        index = self.unity.switcher.selection_index
+
+        pos = self.mouse.position()
+        self.mouse.move(pos[0] + 5, pos[1] + 5)
+
+        # Assert moving the mouse does not change the selection
+        self.assertThat(self.unity.switcher.selection_index, Eventually(Equals(index)))
+
+        # Also nice to know clicking still works, even without selection
+        self.mouse.click()
+
+        self.assertProperty(char_win2, is_focused=True)
+
     def test_mouse_highlights_switcher_deatil_icons_motion(self):
         """
         Gather the cords of all the detail icons, move the mouse through each
@@ -597,15 +630,9 @@
         self.unity.switcher.initiate(SwitcherMode.DETAIL)
         self.addCleanup(self.unity.switcher.terminate)
 
-        offset = self.unity.switcher.view.spread_offset
-        cords = []
-
+        index = 0;
         for icon in self.unity.switcher.view.detail_icons:
-            cords.append((icon.x + offset, icon.y + offset))
-
-        index = 0;
-        for cord in cords:
-          self.mouse.move(cord[0], cord[1])
+          self.unity.switcher.view.move_over_detail_icon(index)
           self.assertThat(index, Equals(self.unity.switcher.detail_selection_index))
           index += 1
 
@@ -621,11 +648,7 @@
         self.unity.switcher.initiate(SwitcherMode.DETAIL)
         self.addCleanup(self.unity.switcher.terminate)
 
-        offset = self.unity.switcher.view.spread_offset
-        x = self.unity.switcher.view.detail_icons[0].x + offset
-        y = self.unity.switcher.view.detail_icons[0].y + offset
-
-        self.mouse.move(x,y)
+        self.unity.switcher.view.move_over_detail_icon(0);
         self.mouse.click()
 
         self.assertProperty(char_win1, is_focused=True)