Merge lp:~rperier/unity/command-lens-switching into lp:unity

[Romain Perier]

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

Unmerged revisions

3084. By Romain Perier on 2013-02-01

Improving autopilot test

3083. By Romain Perier on 2013-01-29

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

=== modified file 'dash/DashView.cpp'
--- dash/DashView.cpp	2013-01-29 00:32:06 +0000
+++ dash/DashView.cpp	2013-02-01 10:38:35 +0000
@@ -1121,7 +1121,10 @@
   }
   else if (/* visible_ && */ handled_type == NOT_HANDLED)
   {
-    ubus_manager_.SendMessage(UBUS_PLACE_VIEW_CLOSE_REQUEST, NULL,
+    if (lens_bar_->GetActiveLensId() != id)
+      lens_bar_->Activate(id);
+    else
+      ubus_manager_.SendMessage(UBUS_PLACE_VIEW_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-02-01 10:38:35 +0000
@@ -64,6 +64,8 @@
 
   sigc::signal<void, std::string const&> lens_activated;
 
+  std::string GetActiveLensId() const;
+
 private:
   void SetupBackground();
   void SetupLayout();
@@ -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-01-28 11:00:19 +0000
+++ tests/autopilot/unity/tests/test_dash.py	2013-02-01 10:38:35 +0000
@@ -68,6 +68,11 @@
         self.dash.ensure_visible()
         self.dash.reveal_command_lens()
         self.assertThat(self.dash.visible, Eventually(Equals(False)))
+	# SWitch from command lens to the dash without closing the dash
+	self.dash.ensure_hidden()
+	self.dash.reveal_command_lens()
+	self.dash.ensure_visible()
+	self.assertThat(self.dash.active_lens, Eventually(Equals('home.lens')))
 
     def test_alt_f4_close_dash(self):
         """Dash must close on alt+F4."""