Merge lp:~3v1n0/unity/super-tab-improvements into lp:unity

[Marco Trevisan (Treviño)]

Improved the Super+Tab switcher making possible to escape from it using the Esc key and making the selection circular.

This branch mostly fixes bug #919018 and bug #919019 as requested by design on recent updates of the main bug #891620

[Sam Spilsbury (smspillaz)]

49 - if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
50 + if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
51 + {

That should test for > 0. In the diff it initially looked like you were changing a + to a - :)

52 _launcher_drag_delta += (_icon_size + _space_between_icons);
53 + }
54 + else if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)
55 + {
56 + _launcher_drag_delta -= (*it)->GetCenter().y + _icon_size/2 +
57 + _space_between_icons - GetGeometry().height;
58 + }
59 }

Other than that. Approve

review: Approve

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

> 49 - if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
> 50 + if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
> 51 + {
>
> That should test for > 0. In the diff it initially looked like you were
> changing a + to a - :)

What do you mean? You'd prefer to move GetGeometry().y to the left side?
The old code contained a copy&paste typo because there was a "+ -" operator that is just a "-" in fact.

[Sam Spilsbury (smspillaz)]

> > 49 - if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
> > 50 + if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
> > 51 + {
> >
> > That should test for > 0. In the diff it initially looked like you were
> > changing a + to a - :)
>
> What do you mean? You'd prefer to move GetGeometry().y to the left side?
> The old code contained a copy&paste typo because there was a "+ -" operator
> that is just a "-" in fact.

Oh whoops, looks like I didn't see the <.

The typo threw me off :)

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

This branch has been merged on lp:~unity-team/unity/unity.multi-launcher so let's close this review.

=== modified file 'manual-tests/SuperTab.txt'
--- manual-tests/SuperTab.txt	2012-01-20 02:25:29 +0000
+++ manual-tests/SuperTab.txt	2012-01-20 02:25:29 +0000
@@ -42,3 +42,36 @@
 Outcome:
   Super+Tab switcher is initialized and the shortcut hint overlay is not shown
   even keeping only super pressed until releasing it and pressing it again.
+
+Super Tab switcher cycling over launcher applications
+-----------------------------------------------------
+This test shows how the Super+Tab switcher should use a circular selection.
+
+#. Start with a clean screen
+#. Press Super+Tab multiple times to make the selection to reach the bottom
+   launcher icon
+#. Press Tab again
+
+Outcome:
+  The selection should go from the last launcher icon, to the first one.
+  The launcher view should be expanded/moved if needed to show the highlighted item.
+
+#. Start with a clean screen
+#. Press Super+Tab once to make the first launcher icon to highlight
+#. Press Shift+Tab while keeping Super pressed
+
+Outcome:
+  The selection should go from the first launcher icon to the last one.
+  The launcher view should be expanded/moved if needed to show the highlighted item.
+
+Escaping from Super Tab switcher
+--------------------------------
+This test shows how to escape from the Super+Tab switcher.
+
+#. Start with a clean screen
+#. Press Super+Tab one or more times to make the launcher switcher to initialize
+#. Press the Escape key when still pressing Super
+
+Outcome:
+  The Super Tab launcher switcher should terminate without performing any action
+  The previously selected launcher item should be deselected.
 
=== modified file 'plugins/unityshell/src/Launcher.cpp'
--- plugins/unityshell/src/Launcher.cpp	2012-01-18 11:16:38 +0000
+++ plugins/unityshell/src/Launcher.cpp	2012-01-20 02:25:29 +0000
@@ -2606,9 +2606,17 @@
     {
       _current_icon_index = temp_current_icon_index;
 
-      if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
+      if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
+      {
         _launcher_drag_delta += (_icon_size + _space_between_icons);
+      }
+      else if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)
+      {
+        _launcher_drag_delta -= (*it)->GetCenter().y + _icon_size/2 +
+                                _space_between_icons - GetGeometry().height;
+      }
     }
+
     EnsureAnimation();
     selection_change.emit();
   }
@@ -2634,7 +2642,13 @@
       _current_icon_index = temp_current_icon_index;
 
       if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)
+      {
         _launcher_drag_delta -= (_icon_size + _space_between_icons);
+      }
+      else if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
+      {
+        _launcher_drag_delta += GetGeometry().y - ((*it)->GetCenter().y - _icon_size);
+      }
     }
 
     EnsureAnimation();
@@ -2684,6 +2698,15 @@
   selection_change.emit();
 }
 
+void Launcher::KeySwitcherCancel()
+{
+  if (!_key_switcher_activated)
+    return;
+
+  _current_icon_index = -1;
+  KeySwitcherTerminate();
+}
+
 bool Launcher::KeySwitcherIsActive()
 {
   return _key_switcher_activated;
@@ -2694,6 +2717,11 @@
   if (!_key_switcher_activated)
     return;
 
+  if (_current_icon_index == _model->Size() - 1)
+  {
+    _current_icon_index = -1;
+  }
+
   SelectNextIcon();
 }
 
@@ -2702,6 +2730,11 @@
   if (!_key_switcher_activated)
     return;
 
+  if (_current_icon_index == 0)
+  {
+    _current_icon_index = _model->Size();
+  }
+
   SelectPreviousIcon();
 }
 
@@ -2861,13 +2894,9 @@
     _start_dragicon_handle = g_timeout_add(START_DRAGICON_DURATION, &Launcher::StartIconDragTimeout, this);
 
     launcher_icon->mouse_down.emit(nux::GetEventButton(button_flags));
-
-    if (_key_switcher_activated)
-    {
-      _current_icon_index = -1;
-      KeySwitcherTerminate();
-    }
   }
+
+  KeySwitcherCancel();
 }
 
 void Launcher::MouseUpLogic(int x, int y, unsigned long button_flags, unsigned long key_flags)
 
=== modified file 'plugins/unityshell/src/Launcher.h'
--- plugins/unityshell/src/Launcher.h	2012-01-17 15:41:30 +0000
+++ plugins/unityshell/src/Launcher.h	2012-01-20 02:25:29 +0000
@@ -210,6 +210,7 @@
 
   void KeySwitcherActivate();
   void KeySwitcherTerminate();
+  void KeySwitcherCancel();
   bool KeySwitcherIsActive();
   void KeySwitcherNext();
   void KeySwitcherPrevious();
 
=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-01-20 02:25:29 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-01-20 02:25:29 +0000
@@ -107,6 +107,7 @@
   , _edge_trigger_handle(0)
   , _redraw_handle(0)
   , _edge_pointerY(0)
+  , _escape_action(nullptr)
   , newFocusedWindow(nullptr)
   , doShellRepaint(false)
   , allowWindowPaint(false)
@@ -423,6 +424,25 @@
     _shortcut_actions.push_back(action);
 }
 
+void UnityScreen::EnableCancelAction(bool enabled, int modifiers)
+{
+  if (enabled)
+  {
+    /* Create a new keybinding for the Escape key and the current modifiers */
+    CompAction::KeyBinding binding(9, modifiers);
+
+    _escape_action = CompActionPtr(new CompAction());
+    _escape_action->setKey(binding);
+
+    screen->addAction(_escape_action.get());
+  }
+  else if (!enabled && _escape_action.get())
+  {
+    screen->removeAction(_escape_action.get());
+    _escape_action = nullptr;
+  }
+}
+
 void UnityScreen::nuxPrologue()
 {
   /* Vertex lighting isn't used in Unity, we disable that state as it could have
@@ -1098,6 +1118,13 @@
         launcher.startKeyNavMode();
       _key_nav_mode_requested = false;
       break;
+    case ButtonPress:
+      if (super_keypressed_)
+      {
+        launcher.KeySwitcherCancel();
+        EnableCancelAction(false);
+      }
+      break;
     case KeyPress:
     {
       KeySym key_sym;
@@ -1109,7 +1136,7 @@
         // we should just say "key_string[1] = 0" because that is the only
         // thing that could possibly make sense here.
         key_string[result] = 0;
-        if (super_keypressed_)
+        if (super_keypressed_ && key_sym != XK_Escape)
         {
           g_idle_add([] (gpointer data) -> gboolean {
             auto self = static_cast<UnityScreen*>(data);
@@ -1270,9 +1297,11 @@
                                            CompAction::State state,
                                            CompOption::Vector& options)
 {
+  if (state & CompAction::StateCancel)
+    return false;
+
   super_keypressed_ = false;
   launcher_controller_->launcher().EndKeyShowLauncher();
-  launcher_controller_->launcher().KeySwitcherTerminate();
 
   shortcut_controller_->SetEnabled(enable_shortcut_overlay_);
   shortcut_controller_->Hide();
@@ -1537,21 +1566,43 @@
   Launcher& launcher = launcher_controller_->launcher();
 
   if (!launcher.KeySwitcherIsActive())
+  {
+    EnableCancelAction(true, action->key().modifiers());
     launcher.KeySwitcherActivate();
+  }
   else
+  {
     launcher.KeySwitcherNext();
+  }
 
+  action->setState(action->state() | CompAction::StateTermKey);
   return false;
 }
+
 bool UnityScreen::launcherSwitcherPrevInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options)
 {
   launcher_controller_->launcher().KeySwitcherPrevious();
 
   return false;
 }
+
 bool UnityScreen::launcherSwitcherTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options)
 {
-  launcher_controller_->launcher().KeySwitcherTerminate();
+  Launcher& launcher = launcher_controller_->launcher();
+
+  if (launcher.KeySwitcherIsActive())
+  {
+    if (state & CompAction::StateCancel)
+    {
+      launcher.KeySwitcherCancel();
+    }
+    else
+    {
+      launcher.KeySwitcherTerminate();
+    }
+
+    EnableCancelAction(false);
+  }
 
   return false;
 }
 
=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2012-01-20 02:25:29 +0000
+++ plugins/unityshell/src/unityshell.h	2012-01-20 02:25:29 +0000
@@ -225,6 +225,7 @@
 
   void EnsureSuperKeybindings ();
   void CreateSuperNewAction(char shortcut, bool use_shift=false, bool use_numpad=false);
+  void EnableCancelAction(bool enabled, int modifiers = 0);
 
   static gboolean initPluginActions(gpointer data);
   void initLauncher();
@@ -281,6 +282,7 @@
   typedef std::shared_ptr<CompAction> CompActionPtr;
   typedef std::vector<CompActionPtr> ShortcutActions;
   ShortcutActions _shortcut_actions;
+  CompActionPtr   _escape_action;
   bool            super_keypressed_;
 
   /* keyboard-nav mode */