Merge lp:~brandontschaefer/unity/lp.1108956-fix into lp:unity

[Brandon Schaefer]

DashView: Don't close the dash when switching from the command lens to the home lens

When I open the command lens by pressing ALT+F2, if I press "Super" the dash is closed. It should not be closed, it should switch to the home lens instead. This is the suite of the bug 1019457 .

I already opened a bug for that, this is the bug https://bugs.launchpad.net/ubuntu/+source/unity/+bug/1108956

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

Code looks good (one indentation on line 13 a part).

Thanks a lot!

On tests:

50 +

Please, remove this empty space

49 + self.assertThat(self.dash.visible, Eventually(Equals(False)))

Probably it's better to check this instead:
self.assertThat(self.dash.active_lens, Eventually(Equals('home.lens')))

Also, please add that assert statement also in the test_can_go_from_dash_to_command_lens test.

[Romain Perier (rperier)]

Updated, is it correct now ?

[Łukasz Zemczak (sil2100)]

Hi! I actually think that Marco had something different in mind ;)

So, it was good that there were 2 different tests before, so if you could revert to the previous state it was good.

The thing what we wanted to be changed is using self.assertThat(self.dash.active_lens, Eventually(Equals('home.lens'))) instead of self.assertThat(self.dash.visible, Eventually(Equals(False))) in the test_can_go_from_command_lens_to_dash test. Since this way we'll be checking that when pressing 'Super', that the home lens of the dash will be opened, not some other lens.

By 'Also, please add that assert statement also in the test_can_go_from_dash_to_command_lens test.' Marco probably meant for you to modify the previous test of test_can_go_from_dash_to_command_lens to also check if the command lens is opened. So, a check for self.assertThat(self.dash.active_lens, Eventually(Equals('commands.lens'))) . e.g.

    def test_can_go_from_dash_to_command_lens(self):
        """Switch to command lens without closing the dash."""
        self.dash.ensure_visible()
        self.dash.reveal_command_lens()
        self.assertThat(self.dash.visible, Eventually(Equals(False)))
        self.assertThat(self.dash.active_lens, Eventually(Equals('commands.lens')))
        # Here we make sure that the dash is closed and the commans lens opened

    def test_can_go_from_command_lens_to_dash(self):
        self.dash.reveal_command_lens()
        self.dash.ensure_visible()
        self.assertThat(self.dash.visible, Eventually(Equals(True)))
        self.assertThat(self.dash.active_lens, Eventually(Equals('home.lens')))
        # This way we check both if the dash is opened and the right lens

        # Remember about whitespaces!

Also, make sure you use the right identing. In python whitespaces are valid syntax elements, so make sure you use the same number of spaces as other blocks. Since I see you used tab-spaces, while we use only spaces for python autopilot tests.

review: Needs Fixing

[Łukasz Zemczak (sil2100)]

Besides the testing, code and fix looks fine.

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

Romain? Any news on this? :)

[Brandon Schaefer (brandontschaefer)]

Using this branch:
https://code.launchpad.net/~brandontschaefer/unity/lp.1108956-fix/+merge/156194

[Brandon Schaefer (brandontschaefer)]

Removed my other MP, and just resubmitted over this one. (Thanks Marco for letting me know you can do that!)

[Brandon Schaefer (brandontschaefer)]

This is going to cause some problems with autopilot. The problem being, ensure_hidden() uses the keybinding "dash/reveal", which is set to Super. The problem with this is if we try to hide while in the command lens we will just end up going to the home.lens and not actually hiding... though im running into the problem with trunk unity other lens such as the application lens. So this will cause regressions, so lets fix the AP tests before merging this.

[Brandon Schaefer (brandontschaefer)]

Turns out something was wrong with my system...but I had to use a explicit Super call other was ensure_visible() would fail after already opening the command lens. As the dash is visible, so ensure_visible will skip calling "dash/reveal".

[lp:~brandontschaefer/unity/lp.1108956-fix]

3265. By Brandon Schaefer on 2013-04-02

Release the barrier if there are no launcher/subscribers to that monitor. Fixes: https://bugs.launchpad.net/bugs/1161726.

Approved by PS Jenkins bot, Stephen M. Webb, MC Return.

[Andrea Azzarone (azzar1)]

LGTM.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[lp:~brandontschaefer/unity/lp.1108956-fix]

3266. By Brandon Schaefer on 2013-04-02

DashView: Don't close the dash when switching from the command lens to the home lens. Fixes: https://bugs.launchpad.net/bugs/1108956.

Approved by PS Jenkins bot, Andrea Azzarone.

=== modified file 'dash/DashView.cpp'
--- dash/DashView.cpp	2013-03-19 18:47:01 +0000
+++ dash/DashView.cpp	2013-04-01 16:52:23 +0000
@@ -1135,8 +1135,15 @@
   }
   else if (/* visible_ && */ handled_type == NOT_HANDLED)
   {
-    ubus_manager_.SendMessage(UBUS_OVERLAY_CLOSE_REQUEST, NULL,
-                              glib::Source::Priority::HIGH);
+    if (lens_bar_->GetActiveLensId() != id)
+    {
+      lens_bar_->Activate(id);
+    }
+    else
+    {
+      ubus_manager_.SendMessage(UBUS_OVERLAY_CLOSE_REQUEST, NULL,
+                                glib::Source::Priority::HIGH);
+    }
   }
   else if (/* visible_ && */ handled_type == GOTO_DASH_URI)
   {

=== modified file 'dash/LensBar.h'
--- dash/LensBar.h	2012-12-14 12:14:34 +0000
+++ dash/LensBar.h	2013-04-01 16:52:23 +0000
@@ -61,6 +61,8 @@
   void Activate(std::string id);
   void ActivateNext();
   void ActivatePrevious();
+  
+  std::string GetActiveLensId() const;
 
   sigc::signal<void, std::string const&> lens_activated;
 
@@ -81,7 +83,6 @@
   std::string GetName() const;
   void AddProperties(GVariantBuilder* builder);
 
-  std::string GetActiveLensId() const;
   typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
 
   LensIcons icons_;

=== modified file 'tests/autopilot/unity/tests/test_dash.py'
--- tests/autopilot/unity/tests/test_dash.py	2013-03-14 13:58:33 +0000
+++ tests/autopilot/unity/tests/test_dash.py	2013-04-01 16:52:23 +0000
@@ -73,8 +73,18 @@
         """Switch to command lens without closing the dash."""
         self.unity.dash.ensure_visible()
         self.unity.dash.reveal_command_lens()
+        self.assertThat(self.unity.dash.visible, Eventually(Equals(True)))
         self.assertThat(self.unity.dash.active_lens, Eventually(Equals('commands.lens')))
 
+    def test_can_go_from_command_lens_to_dash(self):
+        """We must be able to go from the command lens to the dash (home lens)."""
+        self.unity.dash.reveal_command_lens()
+
+        # Since the dash is visible we can't use ensure_visible().
+        self.keybinding("dash/reveal", 0.1)
+        self.assertThat(self.unity.dash.visible, Eventually(Equals(True)))
+        self.assertThat(self.unity.dash.active_lens, Eventually(Equals('home.lens')))
+
     def test_alt_f4_close_dash(self):
         """Dash must close on alt+F4."""
         self.unity.dash.ensure_visible()