Merge lp:~azzar1/unity/fix-979686 into lp:unity

[Andrea Azzarone]

Fix inconsistent hover/highlight/clickable area behavior.

== Problem ==
Dash: Inconsistent hover/highlight/clickable area behavior

== Fix ==
Expand the HeaderView.

== Test ==
AP test added. The test doesn't work for me because self.dash.reveal_file_lens() is broken for some reasons on my machine. If I manually show the file lens during the test execution everything is ok.

[Thomi Richards (thomir-deactivatedaccount)]

Hi,

The test works and passes for me on my machine. However, i'd appreciate it if you could change your test a little bit:

First, please replace "should" in your docstring to "must".
56 + def setUp(self):
57 + super(CategoryHeaderTests, self).setUp()
58 + self.dash.reveal_file_lens()
59 + self.lens = self.dash.get_current_lens()

These lines of code don't belong in setUp(). They're not setting up the test environment for the test - they're part of the test execution. I'd be happy if you wanted to change the reveal_foo_lens methods to return the revealed lens, so this is one line instead of two. I'd also be happy if you wanted to put these lines of code into a separate method and call it from the start of your test.

Also, if this test fails, you will be leaving the dash open. Can you please add a :

self.addCleanup(self.dash.ensure_hidden) right after you reveal the application lens please? If your test passes, this is a no-op, so it's quite safe.

Finally, is there a way we can make this test leave the lens in the same state as it found it? Can we un-expand the category header as well? Maybe add a second assert, so we know that clicking in that spot will bot expand and collapse the category header? What do you think?

Cheers,

review: Needs Fixing

[Andrea Azzarone (azzar1)]

> Hi,
>
> The test works and passes for me on my machine. However, i'd appreciate it if
> you could change your test a little bit:
>
> First, please replace "should" in your docstring to "must".
> 56 + def setUp(self):
> 57 + super(CategoryHeaderTests, self).setUp()
> 58 + self.dash.reveal_file_lens()
> 59 + self.lens = self.dash.get_current_lens()
>
> These lines of code don't belong in setUp(). They're not setting up the test
> environment for the test - they're part of the test execution. I'd be happy if
> you wanted to change the reveal_foo_lens methods to return the revealed lens,
> so this is one line instead of two. I'd also be happy if you wanted to put
> these lines of code into a separate method and call it from the start of your
> test.
>
> Also, if this test fails, you will be leaving the dash open. Can you please
> add a :
>
> self.addCleanup(self.dash.ensure_hidden) right after you reveal the
> application lens please? If your test passes, this is a no-op, so it's quite
> safe.
>
> Finally, is there a way we can make this test leave the lens in the same state
> as it found it? Can we un-expand the category header as well? Maybe add a
> second assert, so we know that clicking in that spot will bot expand and
> collapse the category header? What do you think?
>
>
> Cheers,

Done. What do you think now?

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve

=== modified file 'dash/PlacesGroup.cpp'
--- dash/PlacesGroup.cpp	2012-05-07 22:28:17 +0000
+++ dash/PlacesGroup.cpp	2012-06-11 10:35:23 +0000
@@ -86,8 +86,12 @@
 
   void DrawContent(nux::GraphicsEngine& graphics_engine, bool force_draw)
   {
-    if (GetLayout())
+    if (IsFullRedraw() && GetLayout())
+    {
+      nux::GetPainter().PushPaintLayerStack();
       GetLayout()->ProcessDraw(graphics_engine, force_draw);
+      nux::GetPainter().PopPaintLayerStack();
+    }
   }
 
   bool AcceptKeyNavFocus()
@@ -97,12 +101,11 @@
 
   nux::Area* FindAreaUnderMouse(const nux::Point& mouse_position, nux::NuxEventType event_type)
   {
-    bool mouse_inside = TestMousePointerInclusionFilterMouseWheel(mouse_position, event_type);
-
-    if (mouse_inside == false)
+    if (event_type != nux::EVENT_MOUSE_WHEEL &&
+        TestMousePointerInclusion(mouse_position, event_type))
+      return this;
+    else
       return nullptr;
-
-    return this;
   }
 };
 
@@ -134,7 +137,7 @@
   _group_layout->AddLayout(new nux::SpaceLayout(top_space, top_space, top_space, top_space), 0);
 
   _header_view = new HeaderView(NUX_TRACKER_LOCATION);
-  _group_layout->AddView(_header_view, 0, nux::MINOR_POSITION_TOP, nux::MINOR_SIZE_FIX);
+  _group_layout->AddView(_header_view, 0, nux::MINOR_POSITION_TOP, nux::MINOR_SIZE_FULL);
 
   _header_layout = new nux::HLayout(NUX_TRACKER_LOCATION);
   _header_layout->SetLeftAndRightPadding(style.GetCategoryHeaderLeftPadding(), 0);

=== modified file 'tests/autopilot/unity/emulators/dash.py'
--- tests/autopilot/unity/emulators/dash.py	2012-05-08 16:13:17 +0000
+++ tests/autopilot/unity/emulators/dash.py	2012-06-11 10:35:23 +0000
@@ -103,21 +103,25 @@
         """Reveal the application lense."""
         logger.debug("Revealing application lens with Super+a.")
         self._reveal_lens("lens_reveal/apps", clear_search)
+        return self.view.get_lensview_by_name("applications.lens")
 
     def reveal_music_lens(self, clear_search=True):
         """Reveal the music lense."""
         logger.debug("Revealing music lens with Super+m.")
         self._reveal_lens("lens_reveal/music", clear_search)
+        return self.view.get_lensview_by_name("music.lens")
 
     def reveal_file_lens(self, clear_search=True):
         """Reveal the file lense."""
         logger.debug("Revealing file lens with Super+f.")
         self._reveal_lens("lens_reveal/files", clear_search)
+        return self.view.get_lensview_by_name("files.lens")
 
     def reveal_command_lens(self, clear_search=True):
         """Reveal the 'run command' lens."""
         logger.debug("Revealing command lens with Alt+F2.")
         self._reveal_lens("lens_reveal/command", clear_search)
+        return self.view.get_lensview_by_name("commands.lens")
 
     def _reveal_lens(self, binding_name, clear_search):
         self.keybinding_hold(binding_name)

=== modified file 'tests/autopilot/unity/tests/test_dash.py'
--- tests/autopilot/unity/tests/test_dash.py	2012-05-08 16:13:17 +0000
+++ tests/autopilot/unity/tests/test_dash.py	2012-06-11 10:35:23 +0000
@@ -489,3 +489,26 @@
 
         self.assertThat(self.dash.visible, Eventually(Equals(True)))
 
+
+class CategoryHeaderTests(DashTestCase):
+    """Tests that category headers work.
+    """
+    def test_click_inside_highlight(self):
+        """Clicking into a category highlight must expand/collapse
+        the view.
+        """
+        lens = self.dash.reveal_file_lens()
+        self.addCleanup(self.dash.ensure_hidden)
+
+        category = lens.get_category_by_name("Folders")
+        is_expanded = category.is_expanded
+
+        self.mouse.move(self.dash.view.x + self.dash.view.width / 2,
+                        category.header_y + category.header_height / 2)
+
+        self.mouse.click() 
+        self.assertThat(category.is_expanded, Eventually(Equals(not is_expanded)))
+
+        self.mouse.click()
+        self.assertThat(category.is_expanded, Eventually(Equals(is_expanded)))
+