Merge lp:~3v1n0/unity/alt+tab-scroll-wheel into lp:unity

Proposed by Marco Trevisan (Treviño) on 2012-02-26
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2012-03-12
Approved revision: 2032
Merged at revision: 2090
Proposed branch: lp:~3v1n0/unity/alt+tab-scroll-wheel
Merge into: lp:unity
Diff against target: 277 lines (+137/-13)
4 files modified
plugins/unityshell/src/unityshell.cpp (+46/-8)
plugins/unityshell/src/unityshell.h (+2/-1)
tests/autopilot/autopilot/emulators/unity/switcher.py (+16/-3)
tests/autopilot/autopilot/tests/test_switcher.py (+73/-1)
To merge this branch: bzr merge lp:~3v1n0/unity/alt+tab-scroll-wheel
Reviewer Review Type Date Requested Status
Alex Launi (community) Approve on 2012-03-07
Thomi Richards (community) 2012-02-26 Approve on 2012-02-29
Review via email: mp+94675@code.launchpad.net

Description of the change

Partly fixes bug #824965 enabling scroll wheel on Alt+Tab to switch the active application.

To post a comment you must log in.

Looks good, but with a few things that need fixing.

1) The docstrings for next_icon_mouse and previous_icon_mouse should say: "icon" instead of "application".

2) If possible, we should use constsants to define the mouse buttons in unity. These must be defined in X somewhere?

3)Your tests need to be split up, like this:

107 + def test_switcher_scroll_next(self):
108 + self.set_timeout_setting(False)
109 + sleep(1)
110 +
111 + self.switcher.initiate()
112 + sleep(.2)
113 +
114 + start = self.switcher.get_selection_index()
115 + self.switcher.next_icon_mouse()
116 + sleep(.2)
117 +
118 + end = self.switcher.get_selection_index()
119 + self.assertThat(start, NotEquals(0))
120 + self.assertThat(end, Equals(start + 1))
121 +

def test_switcher_ignores_fast_wheel_down_events(self):
122 + # Quickly repeatead events should be ignored (except the first)
123 + start = self.switcher.get_selection_index()
124 + self.switcher.next_icon_mouse()
125 + self.switcher.next_icon_mouse()
126 + sleep(.2)
127 +
128 + end = self.switcher.get_selection_index()
129 + self.assertThat(end, Equals(start + 1))
130 +
131 + self.switcher.terminate()
132 +

133 + def test_switcher_scroll_prev(self):
134 + self.set_timeout_setting(False)
135 + sleep(1)
136 +
137 + self.switcher.initiate()
138 + sleep(.2)
139 +
140 + start = self.switcher.get_selection_index()
141 + self.switcher.previous_icon_mouse()
142 + sleep(.2)
143 +
144 + end = self.switcher.get_selection_index()
145 + self.assertThat(start, NotEquals(0))
146 + self.assertThat(end, Equals(start - 1))
147 +

def test_switcher_ignores_fast_wheel_down_events(self):
148 + # Quickly repeatead events should be ignored (except the first)
149 + start = self.switcher.get_selection_index()
150 + self.switcher.previous_icon_mouse()
151 + self.switcher.previous_icon_mouse()
152 + sleep(.2)
153 +
154 + end = self.switcher.get_selection_index()
155 + self.assertThat(end, Equals(start - 1))
156 +
157 + self.switcher.terminate()
158 + self.set_timeout_setting(True)
159 +

review: Needs Fixing

Looks good

review: Approve
Alex Launi (alexlauni) wrote :

This currently has conflicts with trunk in plugins/unityshell/src/unityshell.cpp

review: Needs Fixing
Alex Launi (alexlauni) wrote :

nice

review: Approve
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/431/console reported an error when processing this lp:~3v1n0/unity/alt+tab-scroll-wheel branch.
Not merging it.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/unityshell.cpp'
2--- plugins/unityshell/src/unityshell.cpp 2012-03-06 18:24:09 +0000
3+++ plugins/unityshell/src/unityshell.cpp 2012-03-07 17:27:25 +0000
4@@ -93,6 +93,8 @@
5 {
6 // Tap duration in milliseconds.
7 const int ALT_TAP_DURATION = 250;
8+const unsigned int SCROLL_DOWN_BUTTON = 6;
9+const unsigned int SCROLL_UP_BUTTON = 7;
10 } // namespace local
11 } // anon namespace
12
13@@ -124,6 +126,7 @@
14 , dash_is_open_ (false)
15 , grab_index_ (0)
16 , painting_tray_ (false)
17+ , last_scroll_event_(0)
18 , last_hud_show_time_(0)
19 {
20 Timer timer;
21@@ -1340,6 +1343,25 @@
22 EnableCancelAction(false);
23 }
24 break;
25+ case ButtonRelease:
26+ if (switcher_controller_ && switcher_controller_->Visible())
27+ {
28+ XButtonEvent *bev = reinterpret_cast<XButtonEvent*>(event);
29+ if (bev->time - last_scroll_event_ > 150)
30+ {
31+ if (bev->button == Button4 || bev->button == local::SCROLL_UP_BUTTON)
32+ {
33+ switcher_controller_->Prev();
34+ last_scroll_event_ = bev->time;
35+ }
36+ else if (bev->button == Button5 || bev->button == local::SCROLL_DOWN_BUTTON)
37+ {
38+ switcher_controller_->Next();
39+ last_scroll_event_ = bev->time;
40+ }
41+ }
42+ }
43+ break;
44 case KeyPress:
45 {
46 KeySym key_sym;
47@@ -1601,7 +1623,7 @@
48 return true;
49 }
50
51-bool UnityScreen::altTabInitiateCommon(switcher::ShowMode show_mode)
52+bool UnityScreen::altTabInitiateCommon(CompAction* action, switcher::ShowMode show_mode)
53 {
54 if (!grab_index_)
55 grab_index_ = screen->pushGrab (screen->invisibleCursor(), "unity-switcher");
56@@ -1619,6 +1641,14 @@
57 screen->addAction(&optionGetAltTabDetailStop());
58 screen->addAction(&optionGetAltTabLeft());
59
60+ /* Create a new keybinding for scroll buttons and current modifiers */
61+ CompAction scroll_up;
62+ CompAction scroll_down;
63+ scroll_up.setButton(CompAction::ButtonBinding(local::SCROLL_UP_BUTTON, action->key().modifiers()));
64+ scroll_down.setButton(CompAction::ButtonBinding(local::SCROLL_DOWN_BUTTON, action->key().modifiers()));
65+ screen->addAction(&scroll_up);
66+ screen->addAction(&scroll_down);
67+
68 // maybe check launcher position/hide state?
69
70 int device = screen->outputDeviceForPoint (pointerX, pointerY);
71@@ -1655,10 +1685,18 @@
72 screen->removeGrab(grab_index_, NULL);
73 grab_index_ = 0;
74
75- screen->removeAction (&optionGetAltTabRight ());
76- screen->removeAction (&optionGetAltTabDetailStart ());
77- screen->removeAction (&optionGetAltTabDetailStop ());
78- screen->removeAction (&optionGetAltTabLeft ());
79+ screen->removeAction(&optionGetAltTabRight ());
80+ screen->removeAction(&optionGetAltTabDetailStart ());
81+ screen->removeAction(&optionGetAltTabDetailStop ());
82+ screen->removeAction(&optionGetAltTabLeft ());
83+
84+ /* Removing the scroll actions */
85+ CompAction scroll_up;
86+ CompAction scroll_down;
87+ scroll_up.setButton(CompAction::ButtonBinding(local::SCROLL_UP_BUTTON, action->key().modifiers()));
88+ scroll_down.setButton(CompAction::ButtonBinding(local::SCROLL_DOWN_BUTTON, action->key().modifiers()));
89+ screen->removeAction(&scroll_up);
90+ screen->removeAction(&scroll_down);
91
92 bool accept_state = (state & CompAction::StateCancel) == 0;
93 switcher_controller_->Hide(accept_state);
94@@ -1675,7 +1713,7 @@
95 if (switcher_controller_->Visible())
96 switcher_controller_->Next();
97 else
98- altTabInitiateCommon(switcher::ShowMode::CURRENT_VIEWPORT);
99+ altTabInitiateCommon(action, switcher::ShowMode::CURRENT_VIEWPORT);
100
101 action->setState(action->state() | CompAction::StateTermKey);
102 return true;
103@@ -1688,7 +1726,7 @@
104 if (switcher_controller_->Visible())
105 switcher_controller_->Next();
106 else
107- altTabInitiateCommon(switcher::ShowMode::ALL);
108+ altTabInitiateCommon(action, switcher::ShowMode::ALL);
109
110 action->setState(action->state() | CompAction::StateTermKey);
111 return true;
112@@ -1730,7 +1768,7 @@
113 {
114 if (!switcher_controller_->Visible())
115 {
116- altTabInitiateCommon(switcher::ShowMode::CURRENT_VIEWPORT);
117+ altTabInitiateCommon(action, switcher::ShowMode::CURRENT_VIEWPORT);
118 switcher_controller_->Select(1); // always select the current application
119 }
120
121
122=== modified file 'plugins/unityshell/src/unityshell.h'
123--- plugins/unityshell/src/unityshell.h 2012-03-06 18:24:09 +0000
124+++ plugins/unityshell/src/unityshell.h 2012-03-07 17:27:25 +0000
125@@ -192,7 +192,7 @@
126 bool executeCommand(CompAction* action, CompAction::State state, CompOption::Vector& options);
127 bool setKeyboardFocusKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
128
129- bool altTabInitiateCommon(switcher::ShowMode mode);
130+ bool altTabInitiateCommon(CompAction* action, switcher::ShowMode mode);
131 bool altTabTerminateCommon(CompAction* action,
132 CompAction::State state,
133 CompOption::Vector& options);
134@@ -331,6 +331,7 @@
135 CompWindowList fullscreen_windows_;
136 bool painting_tray_;
137 unsigned int tray_paint_mask_;
138+ unsigned int last_scroll_event_;
139 gint64 last_hud_show_time_;
140
141 GLMatrix panel_shadow_matrix_;
142
143=== modified file 'tests/autopilot/autopilot/emulators/unity/switcher.py'
144--- tests/autopilot/autopilot/emulators/unity/switcher.py 2012-03-06 19:03:00 +0000
145+++ tests/autopilot/autopilot/emulators/unity/switcher.py 2012-03-07 17:27:25 +0000
146@@ -12,7 +12,7 @@
147
148 from autopilot.keybindings import KeybindingsHelper
149 from autopilot.emulators.unity import get_state_by_path, make_introspection_object
150-from autopilot.emulators.X11 import Keyboard
151+from autopilot.emulators.X11 import Keyboard, Mouse
152
153 # even though we don't use these directly, we need to make sure they've been
154 # imported so the classes contained are registered with the introspection API.
155@@ -29,6 +29,7 @@
156 def __init__(self):
157 super(Switcher, self).__init__()
158 self._keyboard = Keyboard()
159+ self._mouse = Mouse()
160
161 def initiate(self):
162 """Start the switcher with alt+tab."""
163@@ -78,15 +79,27 @@
164 self.keybinding_release("switcher/reveal_normal")
165
166 def next_icon(self):
167- """Move to the next application."""
168+ """Move to the next icon."""
169 logger.debug("Selecting next item in switcher.")
170 self.keybinding("switcher/next")
171
172 def previous_icon(self):
173- """Move to the previous application."""
174+ """Move to the previous icon."""
175 logger.debug("Selecting previous item in switcher.")
176 self.keybinding("switcher/prev")
177
178+ def next_icon_mouse(self):
179+ """Move to the next icon using the mouse scroll wheel"""
180+ logger.debug("Selecting next item in switcher with mouse scroll wheel.")
181+ self._mouse.press(6)
182+ self._mouse.release(6)
183+
184+ def previous_icon_mouse(self):
185+ """Move to the previous icon using the mouse scroll wheel"""
186+ logger.debug("Selecting previous item in switcher with mouse scroll wheel.")
187+ self._mouse.press(7)
188+ self._mouse.release(7)
189+
190 def show_details(self):
191 """Show detail mode."""
192 logger.debug("Showing details view.")
193
194=== modified file 'tests/autopilot/autopilot/tests/test_switcher.py'
195--- tests/autopilot/autopilot/tests/test_switcher.py 2012-03-06 19:03:00 +0000
196+++ tests/autopilot/autopilot/tests/test_switcher.py 2012-03-07 17:27:25 +0000
197@@ -65,7 +65,79 @@
198
199 self.assertThat(start, NotEquals(0))
200 self.assertThat(end, Equals(start - 1))
201- self.set_timeout_setting(True)
202+
203+ def test_switcher_scroll_next(self):
204+ self.set_timeout_setting(False)
205+ sleep(1)
206+
207+ self.switcher.initiate()
208+ sleep(.2)
209+
210+ start = self.switcher.get_selection_index()
211+ self.switcher.next_icon_mouse()
212+ sleep(.2)
213+
214+ end = self.switcher.get_selection_index()
215+ self.assertThat(start, NotEquals(0))
216+ self.assertThat(end, Equals(start + 1))
217+
218+ self.switcher.terminate()
219+
220+ def test_switcher_scroll_prev(self):
221+ self.set_timeout_setting(False)
222+ sleep(1)
223+
224+ self.switcher.initiate()
225+ sleep(.2)
226+
227+ start = self.switcher.get_selection_index()
228+ self.switcher.previous_icon_mouse()
229+ sleep(.2)
230+
231+ end = self.switcher.get_selection_index()
232+ self.assertThat(start, NotEquals(0))
233+ self.assertThat(end, Equals(start - 1))
234+
235+ self.switcher.terminate()
236+
237+ def test_switcher_scroll_next_ignores_fast_events(self):
238+ self.set_timeout_setting(False)
239+ sleep(1)
240+
241+ self.switcher.initiate()
242+ sleep(.2)
243+
244+ # Quickly repeatead events should be ignored (except the first)
245+ start = self.switcher.get_selection_index()
246+ self.switcher.next_icon_mouse()
247+ self.switcher.next_icon_mouse()
248+ self.switcher.next_icon_mouse()
249+ sleep(.2)
250+
251+ end = self.switcher.get_selection_index()
252+ self.assertThat(start, NotEquals(0))
253+ self.assertThat(end, Equals(start + 1))
254+
255+ self.switcher.terminate()
256+
257+ def test_switcher_scroll_prev_ignores_fast_events(self):
258+ self.set_timeout_setting(False)
259+ sleep(1)
260+
261+ self.switcher.initiate()
262+ sleep(.2)
263+
264+ # Quickly repeatead events should be ignored (except the first)
265+ start = self.switcher.get_selection_index()
266+ self.switcher.previous_icon_mouse()
267+ self.switcher.previous_icon_mouse()
268+ self.switcher.previous_icon_mouse()
269+ sleep(.2)
270+
271+ end = self.switcher.get_selection_index()
272+ self.assertThat(end, Equals(start - 1))
273+
274+ self.switcher.terminate()
275
276 def test_switcher_arrow_key_does_not_init(self):
277 self.set_timeout_setting(False)