Merge lp:~sao/unity/show_launcher_in_navmode into lp:unity

[Oliver Sauder]

[Didier Roche-Tolomelli (didrocks)]

Thanks for your patch proposal :)

So, after reading and testing your branch, there is some side effects, like, press super -> on release one launcher is focused for keynav mode where is shouldn't.

I think as well that MoveFocusToNavMode shouldn't be exposed by launcher.h the API.

Also, I fear some race where the drawing is slow and so, you release the alt + F1 combo before it's also moved on the screen (and so the focus fails).

A more correct approach is to set the focus once the launcher is fully shown, only in _navmod_show_launcher. I can prepare a branch for that (I need another branch to land before) that triggers a signal when the launcher is fully shown (and fully hidden as well).

You can ping me on IRC if you need guidance (my nickname is didrocks). I'll post a follow up on this merge proposal once all the infrastructure will be landed.

By the way, before you finish this and we merge, we need you to sign the Canonical contributer agreement. It's a quick, but necessary step to getting your code into the tree. Luckily you only need to sign it once and it will apply to all other Canonical project contributions you may make in the future. http://www.canonical.com/contributors Make sure to CC David Barth when you send it in.

review: Needs Fixing

[Oliver Sauder (sao)]

I'm not really experienced in compiz (yet... ;) ). So justed wanted to ask if you could point out another similar animation trigger in the unity code which I could follow up to?

[Mirco Müller (macslow)]

The best approach would be to hook into the callback for starting the keyboard-navigation in UnityScreen (unityshell.cpp) check if the launcher is hidden and needs to be shown... and upon exit of the keyboard-navigation (look there for the UBus-callback to LAUNCHER_END_KEY_NAV) make it hide again, if it was hidden before (the start of the keyboard-navigation).

[David Barth (dbarth)]

Just a comment to confirm that Oliver signed the contributor agreement (a long time ago in fact).

[Oliver Sauder (sao)]

MoveFocusToNavMode is not exposed anymore. Side effect fixed so that the key mode is not activated when the Super Key is pressed.

Focus is now moved to the KeyNavMode after the Launcher is fully shown to avoid potential race condition.

[Didier Roche-Tolomelli (didrocks)]

Hey Olivier, thanks for the patch again!

So, I merged it into on of my "fix launcher" branch and we'll get it to trunk and released today :)
I've made some modification to your branch though:
- the OnActionDone is set when you click on an item, not when the launcher is shown, I've change the place of the call.

You can particularly see this with a hidden launcher at start:
press alt + F1 -> no focus (need to use down arrow key).
Also, we want to hide the launcher when a key is pressed, as with the mouse.

- think to prepend a space before () as it's done through the code, we are following GNOME code rules for that.

I've fix those, no worry! Thanks again for your work there.

I will work a little more on some other launcher fixes, then, I'll paste here the link of the merge :)

review: Approve

[Didier Roche-Tolomelli (didrocks)]

ok, merged now. I think the keynav will need some refactoring though so the current solution is temporary (see rev http://bazaar.launchpad.net/~didrocks/unity/misc-launcher-fixes/revision/898 for the merge of your branch relayouted and http://bazaar.launchpad.net/~didrocks/unity/misc-launcher-fixes/revision/899 for some additional fixes)

Thanks again for your work there :)

=== modified file 'src/Launcher.cpp'
--- src/Launcher.cpp	2011-02-25 15:53:14 +0000
+++ src/Launcher.cpp	2011-02-25 22:22:31 +0000
@@ -489,12 +489,20 @@
 {
   _navmod_show_launcher = true;
   EnsureHiddenState ();
+}
 
+void
+Launcher::MoveFocusToKeyNavMode()
+{
   if (_last_icon_index == -1)
-    _current_icon_index = 0;
-  else
-    _current_icon_index = _last_icon_index;
-  NeedRedraw ();
+     _current_icon_index = 0;
+   else
+     _current_icon_index = _last_icon_index;
+   NeedRedraw ();
+
+   ubus_server_send_message (ubus_server_get_default (),
+                             UBUS_LAUNCHER_START_KEY_NAV,
+                             NULL);
 }
 
 void
@@ -1380,6 +1388,10 @@
     Launcher *self = (Launcher*)val;
     self->_mouseover_launcher_locked = false;
     self->SetupAutohideTimer ();
+
+    // move focus to key nav mode when activated
+    if(self->_navmod_show_launcher)
+      self->MoveFocusToKeyNavMode();
 }
 
 void Launcher::ForceHiddenState (bool hidden)

=== modified file 'src/Launcher.h'
--- src/Launcher.h	2011-02-24 14:34:31 +0000
+++ src/Launcher.h	2011-02-25 22:22:31 +0000
@@ -307,6 +307,7 @@
                          int active,
                          nux::Geometry geo);
 
+  void MoveFocusToKeyNavMode();
   void RenderKeyNavHighlight (nux::GraphicsEngine& GfxContext,
                               nux::Geometry        geo);
 

=== modified file 'src/unityshell.cpp'
--- src/unityshell.cpp	2011-02-24 11:09:24 +0000
+++ src/unityshell.cpp	2011-02-25 22:22:31 +0000
@@ -274,7 +274,6 @@
   if (newFocusedWindow != NULL)
   {
     newFocusedWindow->moveInputFocusTo ();
-    launcher->startKeyNavMode ();
   }
 }
 
@@ -283,7 +282,7 @@
                                           CompAction::State  state,
                                           CompOption::Vector &options)
 {
-  startLauncherKeyNav ();
+  launcher->startKeyNavMode();
 
   return false;
 }