Merge lp:~jjed/unity/tooltip-delay into lp:unity

Proposed by Jjed
Status: Rejected
Rejected by: Marco Trevisan (Treviño)
Proposed branch: lp:~jjed/unity/tooltip-delay
Merge into: lp:unity
Diff against target: 433 lines (+149/-15)
10 files modified
plugins/unityshell/src/AbstractLauncherIcon.h (+5/-5)
plugins/unityshell/src/BFBLauncherIcon.cpp (+1/-1)
plugins/unityshell/src/HudLauncherIcon.cpp (+1/-1)
plugins/unityshell/src/Launcher.cpp (+57/-2)
plugins/unityshell/src/Launcher.h (+9/-0)
plugins/unityshell/src/LauncherIcon.cpp (+12/-3)
plugins/unityshell/src/LauncherIcon.h (+3/-1)
plugins/unityshell/src/SimpleLauncherIcon.cpp (+1/-1)
plugins/unityshell/src/SimpleLauncherIcon.h (+1/-1)
tests/autopilot/autopilot/tests/test_launcher.py (+59/-0)
To merge this branch: bzr merge lp:~jjed/unity/tooltip-delay
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Disapprove
Review via email: mp+103120@code.launchpad.net

Description of the change

== Behavior

This branch implements a slight pause before launcher icons display their tooltips. The internal tooltip timer resets on mouse motion; clicking or right-clicking an icon stops the timer until the mouse moves away. After one tooltip displays, other tooltips on the same launcher will pop up instantaneously.

== Implementation

Because tooltip display depends on contextual actions throughout the launcher, the logic for tooltip display has moved from LauncherIcon.cpp to Launcher.cpp. Most mouse-related events in the launcher now trigger changes to the timer state (_show_tooltips_handle).

I explored a separate state machine (Launcher.cpp is already huge and quite intricate), but in practice this just duplicated much of Launcher.cpp into another new module.

== Testing

Added two; they cover the main tooltip behavior. autopilot.tests.test_launcher passes.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

It has few conflicts with trunk, please could you fix it?

review: Needs Fixing
lp:~jjed/unity/tooltip-delay updated
2336. By Jacob Johan Edwards

Remerge with trunk

2337. By Jacob Johan Edwards

Merge with trunk (again)

Revision history for this message
Jjed (jjed) wrote :

Any updates now that the release frenzy has cooled down?

*friendly poke*

lp:~jjed/unity/tooltip-delay updated
2338. By Jacob Johan Edwards

Change tooltip delay to 1s, per design.

It seems .5s, the former value, did not conform to design in lp:#687956.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Hi, I've checked again this branch against the new trunk, and while the behavior looks good (probably the 1sec delay duration can be discussed with design, but the code is fine), the tests need some tuning while the merge conflicts should be resolved:

Instead of self.assertTrue/assertFalse when possible use:
  self.assertThat(introspection.value, Eventually(Equals(True)))

Then move your tests into LauncherTooltipTests that

Also, probabily the HUD / BFB launcher icons special code to override the tooltip show, could be replaced by your codepath, that is more general and I guess can implement LauncherTestCase and define in its setUp self.set_unity_option('launcher_hide_mode', 0)

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

*friendly poke* ;)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Considering the work you're doing on the new branch, I'm going to reject this MR.

review: Disapprove

Unmerged revisions

2338. By Jacob Johan Edwards

Change tooltip delay to 1s, per design.

It seems .5s, the former value, did not conform to design in lp:#687956.

2337. By Jacob Johan Edwards

Merge with trunk (again)

2336. By Jacob Johan Edwards

Remerge with trunk

2335. By Jacob Johan Edwards

Merge with trunk

2334. By Jacob Johan Edwards

Whoops, revert accidental line change.

2333. By Jacob Johan Edwards

Add autopilot test cases for the work in this branch.

2332. By Jacob Johan Edwards

Made LauncherIcon tooltip visibility introspectable for testing.

2331. By Jacob Johan Edwards

Minor cleanup.

2330. By Jacob Johan Edwards

Prevent tooltip flicker after closing quicklists

2329. By Jacob Johan Edwards

Tooltips now disable properly upon icon click.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/AbstractLauncherIcon.h'
2--- plugins/unityshell/src/AbstractLauncherIcon.h 2012-04-25 00:32:14 +0000
3+++ plugins/unityshell/src/AbstractLauncherIcon.h 2012-05-03 23:36:20 +0000
4@@ -209,11 +209,11 @@
5
6 virtual void UnStick() = 0;
7
8- sigc::signal<void, int, int> mouse_down;
9- sigc::signal<void, int, int> mouse_up;
10- sigc::signal<void, int, int> mouse_click;
11- sigc::signal<void, int> mouse_enter;
12- sigc::signal<void, int> mouse_leave;
13+ sigc::signal<void, int, int> mouse_down;
14+ sigc::signal<void, int, int> mouse_up;
15+ sigc::signal<void, int, int> mouse_click;
16+ sigc::signal<void, int, bool> mouse_enter;
17+ sigc::signal<void, int> mouse_leave;
18
19 sigc::signal<void, AbstractLauncherIcon::Ptr> needs_redraw;
20 sigc::signal<void, AbstractLauncherIcon::Ptr> remove;
21
22=== modified file 'plugins/unityshell/src/BFBLauncherIcon.cpp'
23--- plugins/unityshell/src/BFBLauncherIcon.cpp 2012-04-19 22:46:07 +0000
24+++ plugins/unityshell/src/BFBLauncherIcon.cpp 2012-05-03 23:36:20 +0000
25@@ -45,7 +45,7 @@
26
27 background_color_ = nux::color::White;
28
29- mouse_enter.connect([&](int m) { ubus_manager_.SendMessage(UBUS_DASH_ABOUT_TO_SHOW, NULL); });
30+ mouse_enter.connect([&](int m, bool t) { ubus_manager_.SendMessage(UBUS_DASH_ABOUT_TO_SHOW, NULL); });
31 ubus_manager_.RegisterInterest(UBUS_OVERLAY_SHOWN, sigc::bind(sigc::mem_fun(this, &BFBLauncherIcon::OnOverlayShown), true));
32 ubus_manager_.RegisterInterest(UBUS_OVERLAY_HIDDEN, sigc::bind(sigc::mem_fun(this, &BFBLauncherIcon::OnOverlayShown), false));
33 }
34
35=== modified file 'plugins/unityshell/src/HudLauncherIcon.cpp'
36--- plugins/unityshell/src/HudLauncherIcon.cpp 2012-04-24 13:22:42 +0000
37+++ plugins/unityshell/src/HudLauncherIcon.cpp 2012-05-03 23:36:20 +0000
38@@ -72,7 +72,7 @@
39 ubus_manager_.RegisterInterest(UBUS_OVERLAY_SHOWN, sigc::bind(sigc::mem_fun(this, &HudLauncherIcon::OnOverlayShown), true));
40 ubus_manager_.RegisterInterest(UBUS_OVERLAY_HIDDEN, sigc::bind(sigc::mem_fun(this, &HudLauncherIcon::OnOverlayShown), false));
41
42- mouse_enter.connect([&](int m) { ubus_manager_.SendMessage(UBUS_DASH_ABOUT_TO_SHOW); });
43+ mouse_enter.connect([&](int m, bool t) { ubus_manager_.SendMessage(UBUS_DASH_ABOUT_TO_SHOW); });
44 }
45
46 void HudLauncherIcon::SetHideMode(LauncherHideMode hide_mode)
47
48=== modified file 'plugins/unityshell/src/Launcher.cpp'
49--- plugins/unityshell/src/Launcher.cpp 2012-04-24 22:21:03 +0000
50+++ plugins/unityshell/src/Launcher.cpp 2012-05-03 23:36:20 +0000
51@@ -100,6 +100,7 @@
52 const int ANIM_DURATION = 200;
53 const int ANIM_DURATION_LONG = 350;
54 const int START_DRAGICON_DURATION = 250;
55+const int SHOW_TOOLTIPS_DELAY = 1000;
56
57 const int MOUSE_DEADZONE = 15;
58 const float DRAG_OUT_PIXELS = 300.0f;
59@@ -220,6 +221,7 @@
60 _start_dragicon_handle = 0;
61 _dnd_check_handle = 0;
62 _strut_hack_handle = 0;
63+ _show_tooltips_handle = 0;
64 _last_reveal_progress = 0;
65
66 _shortcuts_shown = false;
67@@ -229,6 +231,8 @@
68 _render_drag_window = false;
69 _drag_edge_touching = false;
70 _steal_drag = false;
71+ _tooltips = false;
72+ _preempt_tooltips = false;
73 _last_button_press = 0;
74 _selection_atom = 0;
75 _drag_out_id = 0;
76@@ -310,6 +314,8 @@
77 g_source_remove(_launcher_animation_timeout);
78 if (_strut_hack_handle)
79 g_source_remove(_strut_hack_handle);
80+ if (_show_tooltips_handle)
81+ g_source_remove(_show_tooltips_handle);
82
83 if (_on_data_collected_connection.connected())
84 _on_data_collected_connection.disconnect();
85@@ -409,6 +415,27 @@
86 .add("shortcuts_shown", _shortcuts_shown);
87 }
88
89+void Launcher::ResetTooltipDelay()
90+{
91+ if (_show_tooltips_handle)
92+ g_source_remove(_show_tooltips_handle);
93+ _show_tooltips_handle = 0;
94+}
95+
96+void Launcher::DisableTooltips()
97+{
98+ ResetTooltipDelay();
99+ _tooltips = false;
100+ _preempt_tooltips = true;
101+ if (_icon_under_mouse)
102+ _icon_under_mouse->mouse_enter.emit(monitor, false);
103+}
104+
105+void Launcher::EnableTooltips()
106+{
107+ _preempt_tooltips = false;
108+}
109+
110 void Launcher::SetMousePosition(int x, int y)
111 {
112 bool beyond_drag_threshold = MouseBeyondDragThreshold();
113@@ -1680,6 +1707,9 @@
114 DesaturateIcons();
115 }
116
117+ ResetTooltipDelay();
118+ _tooltips = false;
119+
120 EnsureAnimation();
121 }
122
123@@ -2107,6 +2137,16 @@
124 EnsureAnimation();
125 }
126
127+gboolean Launcher::ShowTooltipsTimeout(gpointer data)
128+{
129+ Launcher* self = (Launcher*) data;
130+ if (self->_icon_under_mouse)
131+ {
132+ self->_tooltips = true;
133+ self->_icon_under_mouse->mouse_enter.emit(self->monitor, true);
134+ }
135+ return false;
136+}
137
138 gboolean Launcher::StartIconDragTimeout(gpointer data)
139 {
140@@ -2269,6 +2309,7 @@
141 {
142 _last_button_press = nux::GetEventButton(button_flags);
143 SetMousePosition(x, y);
144+ ResetTooltipDelay();
145
146 MouseDownLogic(x, y, button_flags, key_flags);
147 EnsureAnimation();
148@@ -2278,6 +2319,7 @@
149 {
150 SetMousePosition(x, y);
151 nux::Geometry geo = GetGeometry();
152+ ResetTooltipDelay();
153
154 MouseUpLogic(x, y, button_flags, key_flags);
155 ResetMouseDragState();
156@@ -2344,6 +2386,7 @@
157 {
158 SetMousePosition(x, y);
159 SetStateMouseOverLauncher(true);
160+ ResetTooltipDelay();
161
162 EventLogic();
163 EnsureAnimation();
164@@ -2353,7 +2396,7 @@
165 {
166 SetMousePosition(x, y);
167 SetStateMouseOverLauncher(false);
168- //AbstractLauncherIcon::SetSkipTooltipDelay(false);
169+ ResetTooltipDelay();
170
171 EventLogic();
172 EnsureAnimation();
173@@ -2362,6 +2405,7 @@
174 void Launcher::RecvMouseMove(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags)
175 {
176 SetMousePosition(x, y);
177+ ResetTooltipDelay();
178
179 if (!_hidden)
180 {
181@@ -2397,6 +2441,7 @@
182 _launcher_drag_delta += 10;
183 }
184
185+ ResetTooltipDelay();
186 EnsureAnimation();
187 }
188
189@@ -2466,6 +2511,7 @@
190 {
191 _hide_machine->SetQuirk(LauncherHideMachine::QUICKLIST_OPEN, true);
192 _hover_machine->SetQuirk(LauncherHoverMachine::QUICKLIST_OPEN, true);
193+ DisableTooltips();
194 EventLogic();
195 EnsureAnimation();
196 }
197@@ -2503,16 +2549,24 @@
198 launcher_icon = MouseIconIntersection(_mouse_position.x, _mouse_position.y);
199 }
200
201+ // if hovering and tooltips are inactive, set a delay timer for them to appear
202+ if (launcher_icon && !_tooltips && !_show_tooltips_handle) {
203+ if (_icon_under_mouse != launcher_icon)
204+ EnableTooltips();
205+ if (!_preempt_tooltips)
206+ _show_tooltips_handle = g_timeout_add(SHOW_TOOLTIPS_DELAY, &Launcher::ShowTooltipsTimeout, this);
207+ }
208
209 if (_icon_under_mouse && (_icon_under_mouse != launcher_icon))
210 {
211 _icon_under_mouse->mouse_leave.emit(monitor);
212 _icon_under_mouse = nullptr;
213+ EnableTooltips();
214 }
215
216 if (launcher_icon && (_icon_under_mouse != launcher_icon))
217 {
218- launcher_icon->mouse_enter.emit(monitor);
219+ launcher_icon->mouse_enter.emit(monitor, _tooltips);
220 _icon_under_mouse = launcher_icon;
221
222 _hide_machine->SetQuirk(LauncherHideMachine::LAST_ACTION_ACTIVATE, false);
223@@ -2551,6 +2605,7 @@
224 if (_icon_mouse_down && (_icon_mouse_down == launcher_icon))
225 {
226 _icon_mouse_down->mouse_up.emit(nux::GetEventButton(button_flags), monitor);
227+ DisableTooltips();
228
229 if (GetActionState() == ACTION_NONE)
230 {
231
232=== modified file 'plugins/unityshell/src/Launcher.h'
233--- plugins/unityshell/src/Launcher.h 2012-04-03 22:45:21 +0000
234+++ plugins/unityshell/src/Launcher.h 2012-05-03 23:36:20 +0000
235@@ -197,6 +197,11 @@
236 static gboolean AnimationTimeout(gpointer data);
237 static gboolean StrutHack(gpointer data);
238 static gboolean StartIconDragTimeout(gpointer data);
239+ static gboolean ShowTooltipsTimeout(gpointer data);
240+
241+ void ResetTooltipDelay();
242+ void EnableTooltips();
243+ void DisableTooltips();
244
245 void SetMousePosition(int x, int y);
246
247@@ -364,6 +369,7 @@
248 guint _start_dragicon_handle;
249 guint _dnd_check_handle;
250 guint _strut_hack_handle;
251+ guint _show_tooltips_handle;
252
253 nux::Point2 _mouse_position;
254 nux::BaseWindow* _parent;
255@@ -406,6 +412,9 @@
256
257 bool _initial_drag_animation;
258
259+ bool _tooltips;
260+ bool _preempt_tooltips;
261+
262 UBusManager ubus;
263
264 nux::Color _background_color;
265
266=== modified file 'plugins/unityshell/src/LauncherIcon.cpp'
267--- plugins/unityshell/src/LauncherIcon.cpp 2012-04-24 18:09:33 +0000
268+++ plugins/unityshell/src/LauncherIcon.cpp 2012-05-03 23:36:20 +0000
269@@ -201,6 +201,7 @@
270 .add("related_windows", static_cast<unsigned int>(Windows().size()))
271 .add("icon_type", _icon_type)
272 .add("tooltip_text", tooltip_text())
273+ .add("tooltip_visible", TooltipVisible())
274 .add("sort_priority", _sort_priority)
275 .add("shortcut", _shortcut)
276 .add("monitors_visibility", g_variant_builder_end(&monitors_builder))
277@@ -518,7 +519,7 @@
278 }
279
280 void
281-LauncherIcon::RecvMouseEnter(int monitor)
282+LauncherIcon::RecvMouseEnter(int monitor, bool show_tooltip)
283 {
284 _last_monitor = monitor;
285 if (QuicklistManager::Default()->Current())
286@@ -527,7 +528,10 @@
287 return;
288 }
289
290- ShowTooltip();
291+ if (show_tooltip)
292+ ShowTooltip();
293+ else
294+ HideTooltip();
295 }
296
297 void LauncherIcon::RecvMouseLeave(int monitor)
298@@ -653,6 +657,11 @@
299 _tooltip->ShowWindow(false);
300 }
301
302+bool LauncherIcon::TooltipVisible()
303+{
304+ return _tooltip && _tooltip->IsVisible();
305+}
306+
307 gboolean
308 LauncherIcon::OnCenterTimeout(gpointer data)
309 {
310@@ -684,7 +693,7 @@
311
312 if (_quicklist && _quicklist->IsVisible())
313 QuicklistManager::Default()->ShowQuicklist(_quicklist.GetPointer(), tip_x, tip_y);
314- else if (_tooltip && _tooltip->IsVisible())
315+ else if (TooltipVisible())
316 _tooltip->ShowTooltipWithTipAt(tip_x, tip_y);
317 }
318
319
320=== modified file 'plugins/unityshell/src/LauncherIcon.h'
321--- plugins/unityshell/src/LauncherIcon.h 2012-04-24 22:21:03 +0000
322+++ plugins/unityshell/src/LauncherIcon.h 2012-05-03 23:36:20 +0000
323@@ -72,7 +72,7 @@
324
325 void SetSortPriority(int priority);
326
327- void RecvMouseEnter(int monitor);
328+ void RecvMouseEnter(int monitor, bool show_tooltip);
329
330 void RecvMouseLeave(int monitor);
331
332@@ -86,6 +86,8 @@
333
334 void ShowTooltip();
335
336+ bool TooltipVisible();
337+
338 bool OpenQuicklist(bool select_first_item = false, int monitor = -1);
339
340 void SetCenter(nux::Point3 center, int parent_monitor, nux::Geometry parent_geo);
341
342=== modified file 'plugins/unityshell/src/SimpleLauncherIcon.cpp'
343--- plugins/unityshell/src/SimpleLauncherIcon.cpp 2012-03-25 22:52:20 +0000
344+++ plugins/unityshell/src/SimpleLauncherIcon.cpp 2012-05-03 23:36:20 +0000
345@@ -81,7 +81,7 @@
346 {
347 }
348
349-void SimpleLauncherIcon::OnMouseEnter(int monitor)
350+void SimpleLauncherIcon::OnMouseEnter(int monitor, bool show_tooltip)
351 {
352 }
353
354
355=== modified file 'plugins/unityshell/src/SimpleLauncherIcon.h'
356--- plugins/unityshell/src/SimpleLauncherIcon.h 2012-03-25 22:52:20 +0000
357+++ plugins/unityshell/src/SimpleLauncherIcon.h 2012-05-03 23:36:20 +0000
358@@ -52,7 +52,7 @@
359 virtual void OnMouseDown(int button, int monitor);
360 virtual void OnMouseUp(int button, int monitor);
361 virtual void OnMouseClick(int button, int monitor);
362- virtual void OnMouseEnter(int monitor);
363+ virtual void OnMouseEnter(int monitor, bool show_tooltip);
364 virtual void OnMouseLeave(int monitor);
365 virtual void ActivateLauncherIcon(ActionArg arg);
366
367
368=== modified file 'tests/autopilot/autopilot/tests/test_launcher.py'
369--- tests/autopilot/autopilot/tests/test_launcher.py 2012-04-29 15:20:38 +0000
370+++ tests/autopilot/autopilot/tests/test_launcher.py 2012-05-03 23:36:20 +0000
371@@ -765,3 +765,62 @@
372 x_fin, y_fin = self.mouse.position()
373 # The launcher should have held the mouse a little bit
374 self.assertThat(x_fin, LessThan(x + width * 1.5))
375+
376+
377+class LauncherTooltipTests(LauncherTestCase):
378+ """Tests whether tooltips display only at appropriate times."""
379+
380+ def test_launcher_tooltip_show(self):
381+ """Tests whether icon tooltips delay showing themselves and,
382+ once shown, whether subsequent icons show them instantly."""
383+ # no tooltips before entering launcher
384+ self.set_unity_option('launcher_hide_mode', 0)
385+ self.launcher_instance.move_mouse_to_right_of_launcher()
386+ icons = self.launcher.model.get_launcher_icons(visible_only=True)
387+ for i in icons:
388+ self.assertFalse(i.tooltip_visible)
389+
390+ # only reveal tooltips after short wait
391+ self.assertEqual(self.get_reveal_behavior(icons[0]), self.DELAYED)
392+
393+ # subsequent tooltips reveal instantly, but hide on exit
394+ a, b = 0, 1
395+ while b < len(icons):
396+ self.mouse.move(icons[b].center_x, icons[b].center_y)
397+ self.assertTrue(icons[b].tooltip_visible)
398+ self.assertFalse(icons[a].tooltip_visible)
399+ a, b = a + 1, b + 1
400+ b -= 1
401+
402+ # leaving launcher clears tooltips, and instant reveal
403+ self.launcher_instance.move_mouse_to_right_of_launcher()
404+ self.assertEqual(self.get_reveal_behavior(icons[b]), self.DELAYED)
405+
406+ def test_launcher_tooltip_disabling(self):
407+ """Tests whether clicking on an icon hides its tooltip."""
408+ self.set_unity_option('launcher_hide_mode', 0)
409+ self.launcher_instance.move_mouse_to_right_of_launcher()
410+ icons = self.launcher.model.get_launcher_icons(visible_only=True)
411+ bfb, other = icons[0], icons[1]
412+ self.assertEqual(self.get_reveal_behavior(bfb), self.DELAYED)
413+
414+ # clicking icon hides its launcher until further input
415+ self.mouse.click()
416+ self.assertEqual(self.get_reveal_behavior(bfb), self.NEVER)
417+ self.mouse.click()
418+
419+ # normal behavior resumes on moving away from icon
420+ self.assertEqual(self.get_reveal_behavior(other), self.DELAYED)
421+ self.assertEqual(self.get_reveal_behavior(bfb), self.INSTANT)
422+
423+ # Tooltip reveal types
424+ (INSTANT, DELAYED, NEVER) = range(3)
425+
426+ def get_reveal_behavior(self, icon):
427+ self.mouse.move(icon.center_x, icon.center_y)
428+ if icon.tooltip_visible:
429+ return self.INSTANT
430+ sleep(1.2)
431+ if icon.tooltip_visible:
432+ return self.DELAYED
433+ return self.NEVER