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

Proposed by Marco Trevisan (Treviño) on 2012-01-20
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2012-01-27
Approved revision: 1844
Merged at revision: 1875
Proposed branch: lp:~3v1n0/unity/super-tab-improvements
Merge into: lp:unity
Prerequisite: lp:~3v1n0/unity/super-tab-switcher-shortcut-interaction
Diff against target: 284 lines (+126/-10)
5 files modified
manual-tests/SuperTab.txt (+33/-0)
plugins/unityshell/src/Launcher.cpp (+36/-7)
plugins/unityshell/src/Launcher.h (+1/-0)
plugins/unityshell/src/unityshell.cpp (+54/-3)
plugins/unityshell/src/unityshell.h (+2/-0)
To merge this branch: bzr merge lp:~3v1n0/unity/super-tab-improvements
Reviewer Review Type Date Requested Status
Sam Spilsbury (community) 2012-01-20 Approve on 2012-01-20
Review via email: mp+89371@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Sam Spilsbury (smspillaz) wrote :

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) wrote :

> 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) wrote :

> > 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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
42Outcome:42Outcome:
43 Super+Tab switcher is initialized and the shortcut hint overlay is not shown43 Super+Tab switcher is initialized and the shortcut hint overlay is not shown
44 even keeping only super pressed until releasing it and pressing it again.44 even keeping only super pressed until releasing it and pressing it again.
45
46Super Tab switcher cycling over launcher applications
47-----------------------------------------------------
48This test shows how the Super+Tab switcher should use a circular selection.
49
50#. Start with a clean screen
51#. Press Super+Tab multiple times to make the selection to reach the bottom
52 launcher icon
53#. Press Tab again
54
55Outcome:
56 The selection should go from the last launcher icon, to the first one.
57 The launcher view should be expanded/moved if needed to show the highlighted item.
58
59#. Start with a clean screen
60#. Press Super+Tab once to make the first launcher icon to highlight
61#. Press Shift+Tab while keeping Super pressed
62
63Outcome:
64 The selection should go from the first launcher icon to the last one.
65 The launcher view should be expanded/moved if needed to show the highlighted item.
66
67Escaping from Super Tab switcher
68--------------------------------
69This test shows how to escape from the Super+Tab switcher.
70
71#. Start with a clean screen
72#. Press Super+Tab one or more times to make the launcher switcher to initialize
73#. Press the Escape key when still pressing Super
74
75Outcome:
76 The Super Tab launcher switcher should terminate without performing any action
77 The previously selected launcher item should be deselected.
4578
=== 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 @@
2606 {2606 {
2607 _current_icon_index = temp_current_icon_index;2607 _current_icon_index = temp_current_icon_index;
26082608
2609 if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)2609 if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
2610 {
2610 _launcher_drag_delta += (_icon_size + _space_between_icons);2611 _launcher_drag_delta += (_icon_size + _space_between_icons);
2612 }
2613 else if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)
2614 {
2615 _launcher_drag_delta -= (*it)->GetCenter().y + _icon_size/2 +
2616 _space_between_icons - GetGeometry().height;
2617 }
2611 }2618 }
2619
2612 EnsureAnimation();2620 EnsureAnimation();
2613 selection_change.emit();2621 selection_change.emit();
2614 }2622 }
@@ -2634,7 +2642,13 @@
2634 _current_icon_index = temp_current_icon_index;2642 _current_icon_index = temp_current_icon_index;
26352643
2636 if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)2644 if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)
2645 {
2637 _launcher_drag_delta -= (_icon_size + _space_between_icons);2646 _launcher_drag_delta -= (_icon_size + _space_between_icons);
2647 }
2648 else if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
2649 {
2650 _launcher_drag_delta += GetGeometry().y - ((*it)->GetCenter().y - _icon_size);
2651 }
2638 }2652 }
26392653
2640 EnsureAnimation();2654 EnsureAnimation();
@@ -2684,6 +2698,15 @@
2684 selection_change.emit();2698 selection_change.emit();
2685}2699}
26862700
2701void Launcher::KeySwitcherCancel()
2702{
2703 if (!_key_switcher_activated)
2704 return;
2705
2706 _current_icon_index = -1;
2707 KeySwitcherTerminate();
2708}
2709
2687bool Launcher::KeySwitcherIsActive()2710bool Launcher::KeySwitcherIsActive()
2688{2711{
2689 return _key_switcher_activated;2712 return _key_switcher_activated;
@@ -2694,6 +2717,11 @@
2694 if (!_key_switcher_activated)2717 if (!_key_switcher_activated)
2695 return;2718 return;
26962719
2720 if (_current_icon_index == _model->Size() - 1)
2721 {
2722 _current_icon_index = -1;
2723 }
2724
2697 SelectNextIcon();2725 SelectNextIcon();
2698}2726}
26992727
@@ -2702,6 +2730,11 @@
2702 if (!_key_switcher_activated)2730 if (!_key_switcher_activated)
2703 return;2731 return;
27042732
2733 if (_current_icon_index == 0)
2734 {
2735 _current_icon_index = _model->Size();
2736 }
2737
2705 SelectPreviousIcon();2738 SelectPreviousIcon();
2706}2739}
27072740
@@ -2861,13 +2894,9 @@
2861 _start_dragicon_handle = g_timeout_add(START_DRAGICON_DURATION, &Launcher::StartIconDragTimeout, this);2894 _start_dragicon_handle = g_timeout_add(START_DRAGICON_DURATION, &Launcher::StartIconDragTimeout, this);
28622895
2863 launcher_icon->mouse_down.emit(nux::GetEventButton(button_flags));2896 launcher_icon->mouse_down.emit(nux::GetEventButton(button_flags));
2864
2865 if (_key_switcher_activated)
2866 {
2867 _current_icon_index = -1;
2868 KeySwitcherTerminate();
2869 }
2870 }2897 }
2898
2899 KeySwitcherCancel();
2871}2900}
28722901
2873void Launcher::MouseUpLogic(int x, int y, unsigned long button_flags, unsigned long key_flags)2902void Launcher::MouseUpLogic(int x, int y, unsigned long button_flags, unsigned long key_flags)
28742903
=== 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 @@
210210
211 void KeySwitcherActivate();211 void KeySwitcherActivate();
212 void KeySwitcherTerminate();212 void KeySwitcherTerminate();
213 void KeySwitcherCancel();
213 bool KeySwitcherIsActive();214 bool KeySwitcherIsActive();
214 void KeySwitcherNext();215 void KeySwitcherNext();
215 void KeySwitcherPrevious();216 void KeySwitcherPrevious();
216217
=== 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 @@
107 , _edge_trigger_handle(0)107 , _edge_trigger_handle(0)
108 , _redraw_handle(0)108 , _redraw_handle(0)
109 , _edge_pointerY(0)109 , _edge_pointerY(0)
110 , _escape_action(nullptr)
110 , newFocusedWindow(nullptr)111 , newFocusedWindow(nullptr)
111 , doShellRepaint(false)112 , doShellRepaint(false)
112 , allowWindowPaint(false)113 , allowWindowPaint(false)
@@ -423,6 +424,25 @@
423 _shortcut_actions.push_back(action);424 _shortcut_actions.push_back(action);
424}425}
425426
427void UnityScreen::EnableCancelAction(bool enabled, int modifiers)
428{
429 if (enabled)
430 {
431 /* Create a new keybinding for the Escape key and the current modifiers */
432 CompAction::KeyBinding binding(9, modifiers);
433
434 _escape_action = CompActionPtr(new CompAction());
435 _escape_action->setKey(binding);
436
437 screen->addAction(_escape_action.get());
438 }
439 else if (!enabled && _escape_action.get())
440 {
441 screen->removeAction(_escape_action.get());
442 _escape_action = nullptr;
443 }
444}
445
426void UnityScreen::nuxPrologue()446void UnityScreen::nuxPrologue()
427{447{
428 /* Vertex lighting isn't used in Unity, we disable that state as it could have448 /* Vertex lighting isn't used in Unity, we disable that state as it could have
@@ -1098,6 +1118,13 @@
1098 launcher.startKeyNavMode();1118 launcher.startKeyNavMode();
1099 _key_nav_mode_requested = false;1119 _key_nav_mode_requested = false;
1100 break;1120 break;
1121 case ButtonPress:
1122 if (super_keypressed_)
1123 {
1124 launcher.KeySwitcherCancel();
1125 EnableCancelAction(false);
1126 }
1127 break;
1101 case KeyPress:1128 case KeyPress:
1102 {1129 {
1103 KeySym key_sym;1130 KeySym key_sym;
@@ -1109,7 +1136,7 @@
1109 // we should just say "key_string[1] = 0" because that is the only1136 // we should just say "key_string[1] = 0" because that is the only
1110 // thing that could possibly make sense here.1137 // thing that could possibly make sense here.
1111 key_string[result] = 0;1138 key_string[result] = 0;
1112 if (super_keypressed_)1139 if (super_keypressed_ && key_sym != XK_Escape)
1113 {1140 {
1114 g_idle_add([] (gpointer data) -> gboolean {1141 g_idle_add([] (gpointer data) -> gboolean {
1115 auto self = static_cast<UnityScreen*>(data);1142 auto self = static_cast<UnityScreen*>(data);
@@ -1270,9 +1297,11 @@
1270 CompAction::State state,1297 CompAction::State state,
1271 CompOption::Vector& options)1298 CompOption::Vector& options)
1272{1299{
1300 if (state & CompAction::StateCancel)
1301 return false;
1302
1273 super_keypressed_ = false;1303 super_keypressed_ = false;
1274 launcher_controller_->launcher().EndKeyShowLauncher();1304 launcher_controller_->launcher().EndKeyShowLauncher();
1275 launcher_controller_->launcher().KeySwitcherTerminate();
12761305
1277 shortcut_controller_->SetEnabled(enable_shortcut_overlay_);1306 shortcut_controller_->SetEnabled(enable_shortcut_overlay_);
1278 shortcut_controller_->Hide();1307 shortcut_controller_->Hide();
@@ -1537,21 +1566,43 @@
1537 Launcher& launcher = launcher_controller_->launcher();1566 Launcher& launcher = launcher_controller_->launcher();
15381567
1539 if (!launcher.KeySwitcherIsActive())1568 if (!launcher.KeySwitcherIsActive())
1569 {
1570 EnableCancelAction(true, action->key().modifiers());
1540 launcher.KeySwitcherActivate();1571 launcher.KeySwitcherActivate();
1572 }
1541 else1573 else
1574 {
1542 launcher.KeySwitcherNext();1575 launcher.KeySwitcherNext();
1576 }
15431577
1578 action->setState(action->state() | CompAction::StateTermKey);
1544 return false;1579 return false;
1545}1580}
1581
1546bool UnityScreen::launcherSwitcherPrevInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options)1582bool UnityScreen::launcherSwitcherPrevInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options)
1547{1583{
1548 launcher_controller_->launcher().KeySwitcherPrevious();1584 launcher_controller_->launcher().KeySwitcherPrevious();
15491585
1550 return false;1586 return false;
1551}1587}
1588
1552bool UnityScreen::launcherSwitcherTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options)1589bool UnityScreen::launcherSwitcherTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options)
1553{1590{
1554 launcher_controller_->launcher().KeySwitcherTerminate();1591 Launcher& launcher = launcher_controller_->launcher();
1592
1593 if (launcher.KeySwitcherIsActive())
1594 {
1595 if (state & CompAction::StateCancel)
1596 {
1597 launcher.KeySwitcherCancel();
1598 }
1599 else
1600 {
1601 launcher.KeySwitcherTerminate();
1602 }
1603
1604 EnableCancelAction(false);
1605 }
15551606
1556 return false;1607 return false;
1557}1608}
15581609
=== 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 @@
225225
226 void EnsureSuperKeybindings ();226 void EnsureSuperKeybindings ();
227 void CreateSuperNewAction(char shortcut, bool use_shift=false, bool use_numpad=false);227 void CreateSuperNewAction(char shortcut, bool use_shift=false, bool use_numpad=false);
228 void EnableCancelAction(bool enabled, int modifiers = 0);
228229
229 static gboolean initPluginActions(gpointer data);230 static gboolean initPluginActions(gpointer data);
230 void initLauncher();231 void initLauncher();
@@ -281,6 +282,7 @@
281 typedef std::shared_ptr<CompAction> CompActionPtr;282 typedef std::shared_ptr<CompAction> CompActionPtr;
282 typedef std::vector<CompActionPtr> ShortcutActions;283 typedef std::vector<CompActionPtr> ShortcutActions;
283 ShortcutActions _shortcut_actions;284 ShortcutActions _shortcut_actions;
285 CompActionPtr _escape_action;
284 bool super_keypressed_;286 bool super_keypressed_;
285287
286 /* keyboard-nav mode */288 /* keyboard-nav mode */