Merge lp:~3v1n0/unity/quicklist-keynav-fixes into lp:unity

Proposed by Marco Trevisan (Treviño) on 2012-04-11
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2012-04-19
Approved revision: 2291
Merged at revision: 2310
Proposed branch: lp:~3v1n0/unity/quicklist-keynav-fixes
Merge into: lp:unity
Diff against target: 1067 lines (+461/-148)
14 files modified
plugins/unityshell/src/AbstractLauncherIcon.h (+1/-1)
plugins/unityshell/src/LauncherIcon.cpp (+3/-3)
plugins/unityshell/src/LauncherIcon.h (+1/-1)
plugins/unityshell/src/MockLauncherIcon.h (+1/-1)
plugins/unityshell/src/QuicklistMenuItem.cpp (+9/-23)
plugins/unityshell/src/QuicklistMenuItem.h (+3/-4)
plugins/unityshell/src/QuicklistMenuItemSeparator.cpp (+6/-0)
plugins/unityshell/src/QuicklistMenuItemSeparator.h (+2/-0)
plugins/unityshell/src/QuicklistView.cpp (+165/-95)
plugins/unityshell/src/QuicklistView.h (+4/-2)
tests/autopilot/autopilot/emulators/unity/quicklist.py (+46/-7)
tests/autopilot/autopilot/keybindings.py (+11/-4)
tests/autopilot/autopilot/tests/test_quicklist.py (+205/-7)
tests/unit/TestQuicklistMenuitems.cpp (+4/-0)
To merge this branch: bzr merge lp:~3v1n0/unity/quicklist-keynav-fixes
Reviewer Review Type Date Requested Status
Thomi Richards (community) 2012-04-11 Approve on 2012-04-18
Review via email: mp+101623@code.launchpad.net

Commit message

QuicklistView: fixed the quicklist key navigation, to make it consistent with Gtk menus navigation.

The prelight of the items is now completely controlled by QuicklistView and never by QuicklistMenuItem, the children can define now when they are slectable.

Description of the change

Quicklist key navigation was broken in multiple aspect, fixed the attached bugs to make it consistent with Gtk menu navigation.

The prelight of the items is now completely controlled by QuicklistView and never by QuicklistMenuItem, the children can define now when they are slectable.

Included Autopilot tests for the QL key navigation, and updated unit-tests for QuicklistMenuItem.

To post a comment you must log in.
Thomi Richards (thomir) wrote :

Hi,

82 +bool
83 +QuicklistMenuItem::GetSelectable()

199 +bool
200 +QuicklistMenuItemSeparator::GetSelectable()

238 +void
239 +QuicklistView::SelectItem(int index)

263 bool
264 -QuicklistView::IsMenuItemSeperator(int index)
265 +QuicklistView::IsMenuItemSelectable(int index)

Coding style says these should be on the same name.

667 + def get_items(self, visible_only=True):
668 + """Returns the quicklist items"""
669 + if visible_only:
670 + return self.get_children_by_type(QuicklistMenuItem, visible=True)
671 + else:
672 + return self.get_children_by_type(QuicklistMenuItem)
673 +
674 @property
675 def items(self):
676 """Individual items in the quicklist."""
677 - return self.get_children_by_type(QuicklistMenuItem)
678 + return self.get_items()
679 +
680 + @property
681 + def selectable_items(self):
682 + """Items that can be selected in the quicklist"""
683 + return filter(lambda it: it.selectable == True, self.items)

This is a bit inconsistent - you have 'get_items' and 'selectable_items' - one a method, one a property. Please make them both the same (either methods or properties, I don't mind which). Also, please don't use 'filter' in autopilot. In most places, list comprehensions are clearer. However, in this case, you don't need either - just pass more keyword filters to get_children_by_type (like this):

@property
def selectable_items(self):
    return self.get_children_by_type(QuicklistMenuItem, visible=True, selectable=True)

704 + Mouse().move(target_x, target_y, animate=False)

Unless you have good reason to, please don't pass 'animate=false'
 - this is only intended to be used in exceptional circumstances.

856 + self.assertThat(self.quicklist, Not(Is(None)))
857 + self.assertThat(self.quicklist.selected_item, Not(Is(None)))

Please avoid negative assertions. Instead of saying "make sure foo is not None", say "Make sure foo is XYZ" - it makes your tests more specific and robust.

A few notes on docstrings:
 1. All your docstrings need a full-stop (period) at the end.
 2. Please try and make your docstrings PEP257 compliant: http://www.python.org/dev/peps/pep-0257/
 3. You can rewrite many of your docstrings to be shorter, and more forceful. For example, instead of writing this:

"""Tests that the quicklist Home key selects the first valid item when
no item is selected
"""

write this:

"""Home key MUST select the first selectable item in a quicklist."""

Note that this uses the word "must" which is better than "should", "might", or "will", and follows more closely the code of the tests themselves.

Other than that, these look good.

review: Needs Fixing
Marco Trevisan (Treviño) (3v1n0) wrote :

> Hi,
>
> 82 +bool
> 83 +QuicklistMenuItem::GetSelectable()
>
> Coding style says these should be on the same name.

Yes I know and I generally do that, but not when I would be inconsistent with the style currently used on the class I'm editing, so I'd prefer not to change it here not to include unneeded changes.
This is something that should be fixed globally on a different coding-style-fixes branch (for Q).

> 667 + def get_items(self, visible_only=True):
> 668 + """Returns the quicklist items"""
> 669 + if visible_only:
> 670 + return self.get_children_by_type(QuicklistMenuItem, visible=True)
> 671 + else:
> 672 + return self.get_children_by_type(QuicklistMenuItem)
> 673 +
> 674 @property
> 675 def items(self):
> 676 """Individual items in the quicklist."""
> 677 - return self.get_children_by_type(QuicklistMenuItem)
> 678 + return self.get_items()
> 679 +
> 680 + @property
> 681 + def selectable_items(self):
> 682 + """Items that can be selected in the quicklist"""
> 683 + return filter(lambda it: it.selectable == True, self.items)
>
>
> This is a bit inconsistent - you have 'get_items' and 'selectable_items' - one
> a method, one a property. Please make them both the same (either methods or
> properties, I don't mind which).

Well, I wanted to keep both for a reason: the get_items() also allow you to get the invisible items of a quicklist, while the property will give you only the "real" result.

> 704 + Mouse().move(target_x, target_y, animate=False)
>
>
> Unless you have good reason to, please don't pass 'animate=false'
> - this is only intended to be used in exceptional circumstances.

I used it to avoid that a quicklist item would have been selected when performing that, adding a possibility to get an invalid result.

> 856 + self.assertThat(self.quicklist, Not(Is(None)))
> 857 + self.assertThat(self.quicklist.selected_item, Not(Is(None)))
>
> Please avoid negative assertions. Instead of saying "make sure foo is not
> None", say "Make sure foo is XYZ" - it makes your tests more specific and
> robust.

I wouldn't have used, but it's just a copy and paste from your code of the same class that I used not to be wrong :D

Thanks for the review, I'll update the branch soon.

Thomi Richards (thomir) wrote :

> > Hi,
> >
> > 82 +bool
> > 83 +QuicklistMenuItem::GetSelectable()
> >
> > Coding style says these should be on the same name.
>
> Yes I know and I generally do that, but not when I would be inconsistent with
> the style currently used on the class I'm editing, so I'd prefer not to change
> it here not to include unneeded changes.
> This is something that should be fixed globally on a different coding-style-
> fixes branch (for Q).

OK, fine with me.
> >
> > This is a bit inconsistent - you have 'get_items' and 'selectable_items' -
> one
> > a method, one a property. Please make them both the same (either methods or
> > properties, I don't mind which).
>
> Well, I wanted to keep both for a reason: the get_items() also allow you to
> get the invisible items of a quicklist, while the property will give you only
> the "real" result.
>

You can still have both - just either make them both properties, or both methods. Probably making them both properties makes more sense.

> > 704 + Mouse().move(target_x, target_y, animate=False)
> >
> >
> > Unless you have good reason to, please don't pass 'animate=false'
> > - this is only intended to be used in exceptional circumstances.
>
> I used it to avoid that a quicklist item would have been selected when
> performing that, adding a possibility to get an invalid result.
>

Fair enough.

> > 856 + self.assertThat(self.quicklist, Not(Is(None)))
> > 857 + self.assertThat(self.quicklist.selected_item, Not(Is(None)))
> >
> > Please avoid negative assertions. Instead of saying "make sure foo is not
> > None", say "Make sure foo is XYZ" - it makes your tests more specific and
> > robust.
>
> I wouldn't have used, but it's just a copy and paste from your code of the
> same class that I used not to be wrong :D
>

Yeah, my code's not perfect either. Negative assertions are something that I've come to realise are really bad very recently, so older code is likely to be wrong.

Cheers,

Marco Trevisan (Treviño) (3v1n0) wrote :

> This is a bit inconsistent - you have 'get_items' and 'selectable_items' - one
> a method, one a property. Please make them both the same (either methods or
> properties, I don't mind which).

Well, I did that because I also wanted to include a method to fetch the invisible items, but I'll remove it since no one uses it yet.

> Also, please don't use 'filter' in autopilot.

Yes you're right, I used it only because at the begginning selectable was a property that I defined only on the emulator, then I moved it to the introspection too, and I forgot to update the code.

I've updated the branch as well, I hope it's fine now ;)

Thomi Richards (thomir) wrote :
Download full text (3.2 KiB)

Hi,

There are still some issues with this I'm afraid:

672 + """Items that can be selected in the quicklist"""

Needs a full-stop at the end of the docstring.

727 + def mouse_click(self, button=1):
728 + logger.debug("Clicking on quicklist item %r", self)
729 + self.mouse_move_to()
730 + sleep(.25)
731 + assert(self.visible)
732 + self._mouse.click(press_duration=.1)
733 + sleep(.25)

Several issues here:
 1) The assert should be at the start of this method, not half way through.
 2) You don't need the sleep() statements, please remove them.
 3) You don't need to specify the click duration. Please just use "mouse.click()".

66 + for icon in self.launcher.model.get_launcher_icons():
867 + if (icon.tooltip_text != self.ql_app.name):
868 + self.ql_launcher.key_nav_next()
869 + else:
870 + self.keybinding("launcher/keynav/open-quicklist")
871 + self.addCleanup(self.keybinding, "launcher/keynav/close-quicklist")
872 + break

Instead of iterating over the model icons, please use the method on the launcher model, like this:

icon = self.launcher.model.get_icon_by_desktop_id(...)

If you use the desktop Id then we won't have issues in didfferent locales.

885 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
899 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
908 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
917 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
931 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
940 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
949 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
958 + self.assertTrue(item.selected)
959 + self.assertThat(self.quicklist.selected_item.id, Equals(item.id))
979 + self.assertTrue(mouse_item.selected)
984 + self.assertTrue(expected_item.selected)
985 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
995 + self.assertTrue(mouse_item.selected)
1000 + self.assertTrue(expected_item.selected)
1001 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
1010 + self.assertTrue(mouse_item.selected)
1017 + self.assertTrue(key_item.selected)
1018 + self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1023 + self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1027 + self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1032 + self.assertTrue(mouse_item.selected)
1033 + self.assertThat(self.quicklist.selected_item.id, Equals(mouse_item.id))

Please use the new Eventually() matcher for all of these. To import it:

from autopilot.matchers import Eventually

to use it, take this:

self.assertThat(self.quicklist.selected_item.id, Equals(mouse_item.id))

and turn it into this:

self.assertThat(self.quicklist.selected_item.id, Eventually(Equals(mouse_item.id)))

Eventually takes any mater, so you can also do things like this:

self.assertThat(self.quicklist.selected_item.id, Eventually(LessThan(mouse_item.id)))

Finally, please remove the sleep() statements. Using the Eventually(...) matcher removes the n...

Read more...

review: Needs Fixing
Marco Trevisan (Treviño) (3v1n0) wrote :

> Needs a full-stop at the end of the docstring.

Ouch!

> 727 + def mouse_click(self, button=1):
> Several issues here:
> 1) The assert should be at the start of this method, not half way through.
> 2) You don't need the sleep() statements, please remove them.
> 3) You don't need to specify the click duration. Please just use
> "mouse.click()".

Done

> 66 + for icon in self.launcher.model.get_launcher_icons():

> Instead of iterating over the model icons, please use the method on the
> launcher model, like this:
>
> icon = self.launcher.model.get_icon_by_desktop_id(...)

I can't... That was a keyboard selection... I must do that.

> If you use the desktop Id then we won't have issues in didfferent locales.

Using the bamf name we don't have these issue... Tested here, with Italian locale ;)

> 885 + self.assertThat(self.quicklist.selected_item.id,
> Equals(expected_item.id))

> Please use the new Eventually() matcher for all of these. To import it:
> Finally, please remove the sleep() statements. Using the Eventually(...)
> matcher removes the need for random sleep statements in your tests.

Done ;)

Thomi Richards (thomir) :
review: Approve
Unity Merger (unity-merger) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

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-03-30 15:24:34 +0000
3+++ plugins/unityshell/src/AbstractLauncherIcon.h 2012-04-19 00:02:18 +0000
4@@ -128,7 +128,7 @@
5
6 virtual void SetSortPriority(int priority) = 0;
7
8- virtual bool OpenQuicklist(bool default_to_first_item = false, int monitor = -1) = 0;
9+ virtual bool OpenQuicklist(bool select_first_item = false, int monitor = -1) = 0;
10
11 virtual void SetCenter(nux::Point3 center, int monitor, nux::Geometry parent_geo) = 0;
12
13
14=== modified file 'plugins/unityshell/src/LauncherIcon.cpp'
15--- plugins/unityshell/src/LauncherIcon.cpp 2012-04-10 00:39:14 +0000
16+++ plugins/unityshell/src/LauncherIcon.cpp 2012-04-19 00:02:18 +0000
17@@ -537,7 +537,7 @@
18 _tooltip->ShowWindow(false);
19 }
20
21-bool LauncherIcon::OpenQuicklist(bool default_to_first_item, int monitor)
22+bool LauncherIcon::OpenQuicklist(bool select_first_item, int monitor)
23 {
24 std::list<DbusmenuMenuitem*> menus = Menus();
25
26@@ -583,8 +583,8 @@
27 _quicklist->AddMenuItem(ql_item);
28 }
29
30- if (default_to_first_item)
31- _quicklist->DefaultToFirstItem();
32+ if (select_first_item)
33+ _quicklist->SelectFirstItem();
34
35 if (monitor < 0)
36 {
37
38=== modified file 'plugins/unityshell/src/LauncherIcon.h'
39--- plugins/unityshell/src/LauncherIcon.h 2012-04-03 22:18:36 +0000
40+++ plugins/unityshell/src/LauncherIcon.h 2012-04-19 00:02:18 +0000
41@@ -86,7 +86,7 @@
42
43 void ShowTooltip();
44
45- bool OpenQuicklist(bool default_to_first_item = false, int monitor = -1);
46+ bool OpenQuicklist(bool select_first_item = false, int monitor = -1);
47
48 void SetCenter(nux::Point3 center, int parent_monitor, nux::Geometry parent_geo);
49
50
51=== modified file 'plugins/unityshell/src/MockLauncherIcon.h'
52--- plugins/unityshell/src/MockLauncherIcon.h 2012-03-30 15:24:34 +0000
53+++ plugins/unityshell/src/MockLauncherIcon.h 2012-04-19 00:02:18 +0000
54@@ -120,7 +120,7 @@
55
56 void SetSortPriority(int priority) { sort_priority_ = priority; }
57
58- bool OpenQuicklist(bool default_to_first_item = false, int monitor = -1)
59+ bool OpenQuicklist(bool select_first_item = false, int monitor = -1)
60 {
61 return false;
62 }
63
64=== modified file 'plugins/unityshell/src/QuicklistMenuItem.cpp'
65--- plugins/unityshell/src/QuicklistMenuItem.cpp 2012-03-14 06:24:18 +0000
66+++ plugins/unityshell/src/QuicklistMenuItem.cpp 2012-04-19 00:02:18 +0000
67@@ -89,10 +89,8 @@
68 this);
69 }
70
71- mouse_down.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseDown));
72 mouse_up.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseUp));
73 mouse_click.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseClick));
74- mouse_move.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseMove));
75 mouse_drag.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseDrag));
76 mouse_enter.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseEnter));
77 mouse_leave.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseLeave));
78@@ -215,6 +213,12 @@
79 DBUSMENU_MENUITEM_PROP_VISIBLE);
80 }
81
82+bool
83+QuicklistMenuItem::GetSelectable()
84+{
85+ return GetVisible() && GetEnabled();
86+}
87+
88 void QuicklistMenuItem::ItemActivated()
89 {
90 if (_debug)
91@@ -333,11 +337,6 @@
92 //self->ItemActivated ();
93 }
94
95-void QuicklistMenuItem::RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags)
96-{
97-
98-}
99-
100 void QuicklistMenuItem::RecvMouseUp(int x, int y, unsigned long button_flags, unsigned long key_flags)
101 {
102 sigMouseReleased.emit(this, x, y);
103@@ -352,11 +351,6 @@
104 sigMouseClick.emit(this, x, y);
105 }
106
107-void QuicklistMenuItem::RecvMouseMove(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags)
108-{
109-
110-}
111-
112 void QuicklistMenuItem::RecvMouseDrag(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags)
113 {
114 sigMouseDrag.emit(this, x, y);
115@@ -364,13 +358,11 @@
116
117 void QuicklistMenuItem::RecvMouseEnter(int x, int y, unsigned long button_flags, unsigned long key_flags)
118 {
119- _prelight = true;
120 sigMouseEnter.emit(this);
121 }
122
123 void QuicklistMenuItem::RecvMouseLeave(int x, int y, unsigned long button_flags, unsigned long key_flags)
124 {
125- _prelight = false;
126 sigMouseLeave.emit(this);
127 }
128
129@@ -484,20 +476,14 @@
130
131 void QuicklistMenuItem::AddProperties(GVariantBuilder* builder)
132 {
133- nux::Geometry abs_geo = GetAbsoluteGeometry();
134-
135 unity::variant::BuilderWrapper(builder)
136- .add("absolute_x", abs_geo.x)
137- .add("absolute_y", abs_geo.y)
138+ .add(GetAbsoluteGeometry())
139 .add("text", _text)
140- .add("x", GetBaseX())
141- .add("y", GetBaseY())
142- .add("width", GetBaseWidth())
143- .add("height", GetBaseHeight())
144 .add("enabled", GetEnabled())
145 .add("active", GetActive())
146 .add("visible", GetVisible())
147- .add("lit", _prelight);
148+ .add("selectable", GetSelectable())
149+ .add("selected", _prelight);
150 }
151
152 } //NAMESPACE
153
154=== modified file 'plugins/unityshell/src/QuicklistMenuItem.h'
155--- plugins/unityshell/src/QuicklistMenuItem.h 2012-03-14 06:24:18 +0000
156+++ plugins/unityshell/src/QuicklistMenuItem.h 2012-04-19 00:02:18 +0000
157@@ -85,11 +85,13 @@
158 virtual bool GetEnabled();
159 virtual bool GetActive();
160 virtual bool GetVisible();
161+ virtual bool GetSelectable();
162
163+protected:
164 // Introspection
165 std::string GetName() const;
166 void AddProperties(GVariantBuilder* builder);
167-protected:
168+
169 static const int ITEM_INDENT_ABS = 16;
170 static const int ITEM_CORNER_RADIUS_ABS = 3;
171 static const int ITEM_MARGIN = 4;
172@@ -116,10 +118,8 @@
173
174 void RecvMouseEnter(int x, int y, unsigned long button_flags, unsigned long key_flags);
175 void RecvMouseLeave(int x, int y, unsigned long button_flags, unsigned long key_flags);
176- void RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags);
177 void RecvMouseUp(int x, int y, unsigned long button_flags, unsigned long key_flags);
178 void RecvMouseClick(int x, int y, unsigned long button_flags, unsigned long key_flags);
179- void RecvMouseMove(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags);
180 void RecvMouseDrag(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags);
181
182 sigc::signal<void, QuicklistMenuItem*> sigMouseEnter;
183@@ -134,7 +134,6 @@
184 nux::Color _color; //!< Item rendering color factor.
185 bool _debug;
186
187-
188 bool _prelight; //!< True when the mouse is over the item.
189
190 void DrawText(nux::CairoGraphics* cairo, int width, int height, nux::Color const& color);
191
192=== modified file 'plugins/unityshell/src/QuicklistMenuItemSeparator.cpp'
193--- plugins/unityshell/src/QuicklistMenuItemSeparator.cpp 2012-03-14 06:24:18 +0000
194+++ plugins/unityshell/src/QuicklistMenuItemSeparator.cpp 2012-04-19 00:02:18 +0000
195@@ -59,6 +59,12 @@
196 {
197 }
198
199+bool
200+QuicklistMenuItemSeparator::GetSelectable()
201+{
202+ return false;
203+}
204+
205 void
206 QuicklistMenuItemSeparator::PreLayoutManagement()
207 {
208
209=== modified file 'plugins/unityshell/src/QuicklistMenuItemSeparator.h'
210--- plugins/unityshell/src/QuicklistMenuItemSeparator.h 2012-03-14 06:24:18 +0000
211+++ plugins/unityshell/src/QuicklistMenuItemSeparator.h 2012-04-19 00:02:18 +0000
212@@ -43,6 +43,8 @@
213
214 ~QuicklistMenuItemSeparator();
215
216+ virtual bool GetSelectable();
217+
218 protected:
219
220 void PreLayoutManagement();
221
222=== modified file 'plugins/unityshell/src/QuicklistView.cpp'
223--- plugins/unityshell/src/QuicklistView.cpp 2012-03-14 06:24:18 +0000
224+++ plugins/unityshell/src/QuicklistView.cpp 2012-04-19 00:02:18 +0000
225@@ -69,7 +69,7 @@
226 , _bottom_padding_correction_single_item(-4)
227 , _offset_correction(-1)
228 , _cairo_text_has_changed(true)
229- , _current_item_index(0)
230+ , _current_item_index(-1)
231 {
232 SetGeometry(nux::Geometry(0, 0, 1, 1));
233
234@@ -138,13 +138,35 @@
235 {
236 }
237
238+void
239+QuicklistView::SelectItem(int index)
240+{
241+ CancelItemsPrelightStatus();
242+ int target_item = -1;
243+
244+ if (IsMenuItemSelectable(index))
245+ {
246+ QuicklistMenuItem* menu_item = GetNthItems(index);
247+
248+ if (menu_item)
249+ {
250+ target_item = index;
251+ menu_item->_prelight = true;
252+ }
253+ }
254+
255+ if (_current_item_index != target_item)
256+ {
257+ _current_item_index = target_item;
258+ selection_change.emit();
259+ QueueDraw();
260+ }
261+}
262+
263 bool
264-QuicklistView::IsMenuItemSeperator(int index)
265+QuicklistView::IsMenuItemSelectable(int index)
266 {
267- QuicklistMenuItem* menu_item = NULL;
268- DbusmenuMenuitem* item = NULL;
269- const gchar* label = NULL;
270- bool result = false;
271+ QuicklistMenuItem* menu_item = nullptr;
272
273 if (index < 0)
274 return false;
275@@ -153,17 +175,7 @@
276 if (!menu_item)
277 return false;
278
279- item = menu_item->_menuItem;
280- if (!item)
281- return false;
282-
283- label = dbusmenu_menuitem_property_get(item, DBUSMENU_MENUITEM_PROP_LABEL);
284- if (!label)
285- result = true;
286- else
287- result = false;
288-
289- return result;
290+ return menu_item->GetSelectable();
291 }
292
293 void
294@@ -175,56 +187,103 @@
295 {
296 switch (key_sym)
297 {
298+ // home or page up (highlight the first menu-hitem)
299+ case NUX_VK_PAGE_UP:
300+ case NUX_VK_HOME:
301+ {
302+ int num_items = GetNumItems();
303+ int target_index = -1;
304+
305+ do
306+ {
307+ ++target_index;
308+ }
309+ while (!IsMenuItemSelectable(target_index) && target_index < num_items);
310+
311+ if (target_index < num_items)
312+ SelectItem(target_index);
313+
314+ break;
315+ }
316+ // end or page down (highlight the last menu-hitem)
317+ case NUX_VK_PAGE_DOWN:
318+ case NUX_VK_END:
319+ {
320+ int target_index = GetNumItems();
321+
322+ do
323+ {
324+ --target_index;
325+ }
326+ while (!IsMenuItemSelectable(target_index) && target_index >= 0);
327+
328+ if (target_index >= 0)
329+ SelectItem(target_index);
330+
331+ break;
332+ }
333 // up (highlight previous menu-item)
334 case NUX_VK_UP:
335 case NUX_KP_UP:
336- // protect against edge-case of first item being a separator
337- if (_current_item_index == 1 && IsMenuItemSeperator(0))
338- break;
339-
340- if (_current_item_index > 0)
341+ {
342+ int target_index = _current_item_index;
343+ bool loop_back = false;
344+
345+ if (target_index <= 0)
346+ target_index = GetNumItems();
347+
348+ do
349 {
350- GetNthItems(_current_item_index)->_prelight = false;
351- _current_item_index--;
352-
353- while (IsMenuItemSeperator(_current_item_index))
354- _current_item_index--;
355-
356- selection_change.emit();
357-
358- GetNthItems(_current_item_index)->_prelight = true;
359- QueueDraw();
360+ --target_index;
361+
362+ // If the first item is not selectable, we must loop from the last one
363+ if (!loop_back && target_index == 0 && !IsMenuItemSelectable(target_index))
364+ {
365+ loop_back = true;
366+ target_index = GetNumItems() - 1;
367+ }
368 }
369+ while (!IsMenuItemSelectable(target_index) && target_index >= 0);
370+
371+ if (target_index >= 0)
372+ SelectItem(target_index);
373+
374 break;
375+ }
376
377 // down (highlight next menu-item)
378 case NUX_VK_DOWN:
379 case NUX_KP_DOWN:
380- // protect against edge-case of last item being a separator
381- if (_current_item_index == (GetNumItems() - 1) && IsMenuItemSeperator(GetNumItems()))
382- break;
383-
384- if (_current_item_index < GetNumItems() - 1)
385+ {
386+ int target_index = _current_item_index;
387+ int num_items = GetNumItems();
388+ bool loop_back = false;
389+
390+ if (target_index >= num_items - 1)
391+ target_index = -1;
392+
393+ do
394 {
395- GetNthItems(_current_item_index)->_prelight = false;
396- _current_item_index++;
397-
398- while (IsMenuItemSeperator(_current_item_index))
399- _current_item_index++;
400-
401- selection_change.emit();
402-
403- GetNthItems(_current_item_index)->_prelight = true;
404- QueueDraw();
405+ ++target_index;
406+
407+ // If the last item is not selectable, we must loop from the first one
408+ if (!loop_back && target_index == num_items - 1 && !IsMenuItemSelectable(target_index))
409+ {
410+ loop_back = true;
411+ target_index = 0;
412+ }
413 }
414+ while (!IsMenuItemSelectable(target_index) && target_index < num_items);
415+
416+ if (target_index < num_items)
417+ SelectItem(target_index);
418+
419 break;
420+ }
421
422 // left (close quicklist, go back to laucher key-nav)
423 case NUX_VK_LEFT:
424 case NUX_KP_LEFT:
425- _current_item_index = 0;
426- selection_change.emit();
427- GetNthItems(_current_item_index)->_prelight = true;
428 Hide();
429 // inform Launcher we switch back to Launcher key-nav
430 ubus_server_send_message(ubus_server_get_default(),
431@@ -234,32 +293,23 @@
432
433 // esc (close quicklist, exit key-nav)
434 case NUX_VK_ESCAPE:
435- _current_item_index = 0;
436- selection_change.emit();
437- GetNthItems(_current_item_index)->_prelight = true;
438 Hide();
439 // inform UnityScreen we leave key-nav completely
440 ubus_server_send_message(ubus_server_get_default(),
441 UBUS_LAUNCHER_END_KEY_NAV,
442 NULL);
443- selection_change.emit();
444 break;
445
446 // <SPACE>, <RETURN> (activate selected menu-item)
447 case NUX_VK_SPACE:
448 case NUX_VK_ENTER:
449 case NUX_KP_ENTER:
450- if (_current_item_index >= 0 && _current_item_index < GetNumItems() &&
451- GetNthItems(_current_item_index)->GetEnabled())
452+ if (IsMenuItemSelectable(_current_item_index))
453 {
454-
455 dbusmenu_menuitem_handle_event(GetNthItems(_current_item_index)->_menuItem,
456 "clicked",
457 NULL,
458 0);
459- _current_item_index = 0;
460- selection_change.emit();
461- GetNthItems(_current_item_index)->_prelight = true;
462 Hide();
463 }
464 break;
465@@ -365,6 +415,12 @@
466 UnGrabKeyboard();
467 //EnableInputWindow (false);
468 ShowWindow(false);
469+
470+ if (_current_item_index != -1)
471+ {
472+ selection_change.emit();
473+ _current_item_index = -1;
474+ }
475 }
476 }
477
478@@ -618,54 +674,53 @@
479 void QuicklistView::RecvItemMouseDrag(QuicklistMenuItem* item, int x, int y)
480 {
481 nux::Geometry geo;
482- std::list<QuicklistMenuItem*>::iterator it;
483- for (it = _item_list.begin(); it != _item_list.end(); it++)
484+
485+ for (auto it : _item_list)
486 {
487- if (!(*it)->GetVisible())
488+ int item_index = GetItemIndex(it);
489+
490+ if (!IsMenuItemSelectable(item_index))
491 continue;
492
493- geo = (*it)->GetGeometry();
494+ geo = it->GetGeometry();
495 geo.width = _item_layout->GetBaseWidth();
496
497 if (geo.IsPointInside(x + item->GetBaseX(), y + item->GetBaseY()))
498 {
499- (*it)->_prelight = true;
500- }
501- else
502- {
503- (*it)->_prelight = false;
504+ SelectItem(item_index);
505 }
506 }
507
508- for (it = _default_item_list.begin(); it != _default_item_list.end(); it++)
509+ for (auto it : _default_item_list)
510 {
511- if (!(*it)->GetVisible())
512+ int item_index = GetItemIndex(it);
513+
514+ if (!IsMenuItemSelectable(item_index))
515 continue;
516
517- geo = (*it)->GetGeometry();
518+ geo = it->GetGeometry();
519 geo.width = _default_item_layout->GetBaseWidth();
520
521 if (geo.IsPointInside(x + item->GetBaseX(), y + item->GetBaseY()))
522 {
523- (*it)->_prelight = true;
524- }
525- else
526- {
527- (*it)->_prelight = false;
528+ SelectItem(item_index);
529 }
530 }
531-
532- NeedRedraw();
533 }
534
535 void QuicklistView::RecvItemMouseEnter(QuicklistMenuItem* item)
536 {
537- NeedRedraw();
538+ int item_index = GetItemIndex(item);
539+
540+ SelectItem(item_index);
541 }
542
543 void QuicklistView::RecvItemMouseLeave(QuicklistMenuItem* item)
544 {
545- NeedRedraw();
546+ int item_index = GetItemIndex(item);
547+
548+ if (item_index < 0 || item_index == _current_item_index)
549+ SelectItem(-1);
550 }
551
552 void QuicklistView::RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags)
553@@ -786,14 +841,37 @@
554 if (_item_list.size() > 0)
555 i = _item_list.size() - 1;
556 std::list<QuicklistMenuItem*>::iterator it;
557- for (it = _item_list.begin(); it != _item_list.end(); i++, it++)
558+ for (it = _default_item_list.begin(); it != _default_item_list.end(); i++, it++)
559 {
560 if (i == index)
561 return *it;
562 }
563 }
564
565- return 0;
566+ return nullptr;
567+}
568+
569+int QuicklistView::GetItemIndex(QuicklistMenuItem* item)
570+{
571+ int index = -1;
572+
573+ for (auto it : _item_list)
574+ {
575+ ++index;
576+
577+ if (it == item)
578+ return index;
579+ }
580+
581+ for (auto it : _default_item_list)
582+ {
583+ ++index;
584+
585+ if (it == item)
586+ return index;
587+ }
588+
589+ return index;
590 }
591
592 QuicklistMenuItemType QuicklistView::GetNthType(int index)
593@@ -801,6 +879,7 @@
594 QuicklistMenuItem* item = GetNthItems(index);
595 if (item)
596 return item->GetItemType();
597+
598 return MENUITEM_TYPE_UNKNOWN;
599 }
600
601@@ -809,13 +888,9 @@
602 return _item_list;
603 }
604
605-void QuicklistView::DefaultToFirstItem()
606+void QuicklistView::SelectFirstItem()
607 {
608- if (GetNumItems() >= 1)
609- {
610- GetNthItems(0)->_prelight = true;
611- QueueDraw();
612- }
613+ SelectItem(0);
614 }
615
616 void ql_tint_dot_hl(cairo_t* cr,
617@@ -1400,15 +1475,10 @@
618
619 void QuicklistView::AddProperties(GVariantBuilder* builder)
620 {
621- nux::Geometry abs_geo = GetAbsoluteGeometry();
622-
623 variant::BuilderWrapper(builder)
624- .add("x", abs_geo.x)
625- .add("y", abs_geo.y)
626+ .add(GetAbsoluteGeometry())
627 .add("base_x", GetBaseX())
628 .add("base_y", GetBaseY())
629- .add("width", GetBaseWidth())
630- .add("height", GetBaseHeight())
631 .add("active", IsVisible());
632 }
633
634
635=== modified file 'plugins/unityshell/src/QuicklistView.h'
636--- plugins/unityshell/src/QuicklistView.h 2012-03-14 06:24:18 +0000
637+++ plugins/unityshell/src/QuicklistView.h 2012-04-19 00:02:18 +0000
638@@ -62,8 +62,9 @@
639 int GetNumItems();
640 QuicklistMenuItem* GetNthItems(int index);
641 QuicklistMenuItemType GetNthType(int index);
642+ int GetItemIndex(QuicklistMenuItem* item);
643 std::list<QuicklistMenuItem*> GetChildren();
644- void DefaultToFirstItem();
645+ void SelectFirstItem();
646
647 void TestMenuItems(DbusmenuMenuitem* root);
648
649@@ -130,7 +131,8 @@
650 //! Check the mouse up event sent by an item. Detect the item where the mous is and emit the appropriate signal.
651 void CheckAndEmitItemSignal(int x, int y);
652
653- bool IsMenuItemSeperator(int index);
654+ void SelectItem(int index);
655+ bool IsMenuItemSelectable(int index);
656
657 //nux::CairoGraphics* _cairo_graphics;
658 int _anchorX;
659
660=== modified file 'tests/autopilot/autopilot/emulators/unity/quicklist.py'
661--- tests/autopilot/autopilot/emulators/unity/quicklist.py 2012-03-06 21:26:11 +0000
662+++ tests/autopilot/autopilot/emulators/unity/quicklist.py 2012-04-19 00:02:18 +0000
663@@ -23,7 +23,12 @@
664 @property
665 def items(self):
666 """Individual items in the quicklist."""
667- return self.get_children_by_type(QuicklistMenuItem)
668+ return self.get_children_by_type(QuicklistMenuItem, visible=True)
669+
670+ @property
671+ def selectable_items(self):
672+ """Items that can be selected in the quicklist."""
673+ return self.get_children_by_type(QuicklistMenuItem, visible=True, selectable=True)
674
675 def get_quicklist_item_by_text(self, text):
676 """Returns a QuicklistMenuItemLabel object with the given text, or None."""
677@@ -40,17 +45,51 @@
678 if not isinstance(item, QuicklistMenuItem):
679 raise TypeError("Item must be a subclass of QuicklistMenuItem")
680
681- logger.debug("Clicking on quicklist item %r", item)
682- mouse = Mouse()
683- mouse.move(self.x + item.x + (item.width / 2),
684- self.y + item.y + (item.height / 2))
685- sleep(0.25)
686- mouse.click()
687+ item.mouse_click()
688+
689+ def move_mouse_to_right(self):
690+ """Moves the mouse outside the quicklist"""
691+ logger.debug("Moving mouse outside the quicklist %r", self)
692+ target_x = self.x + self.width + 10
693+ target_y = self.y + self.height / 2
694+ Mouse().move(target_x, target_y, animate=False)
695+
696+ @property
697+ def selected_item(self):
698+ items = self.get_children_by_type(QuicklistMenuItem, selected=True)
699+ assert(len(items) <= 1)
700+ return items[0] if items else None
701+
702+ @property
703+ def geometry(self):
704+ """Returns a tuple of (x,y,w,h) for the quicklist."""
705+ return (self.x, self.y, self.width, self.height)
706
707
708 class QuicklistMenuItem(UnityIntrospectionObject):
709 """Represents a single item in a quicklist."""
710
711+ def __init__(self, *args, **kwargs):
712+ super(QuicklistMenuItem, self).__init__(*args, **kwargs)
713+ self._mouse = Mouse()
714+
715+ @property
716+ def geometry(self):
717+ """Returns a tuple of (x,y,w,h) for the quicklist item."""
718+ return (self.x, self.y, self.width, self.height)
719+
720+ def mouse_move_to(self):
721+ assert(self.visible)
722+ logger.debug("Moving mouse over quicklist item %r", self)
723+ target_x = self.x + self.width / 2
724+ target_y = self.y + self.height / 2
725+ self._mouse.move(target_x, target_y, rate=20, time_between_events=0.005)
726+
727+ def mouse_click(self, button=1):
728+ logger.debug("Clicking on quicklist item %r", self)
729+ self.mouse_move_to()
730+ self._mouse.click()
731+
732
733 class QuicklistMenuItemLabel(QuicklistMenuItem):
734 """Represents a text label inside a quicklist."""
735
736=== modified file 'tests/autopilot/autopilot/keybindings.py'
737--- tests/autopilot/autopilot/keybindings.py 2012-04-06 01:28:24 +0000
738+++ tests/autopilot/autopilot/keybindings.py 2012-04-19 00:02:18 +0000
739@@ -56,7 +56,14 @@
740 "launcher/switcher/prev": "Shift+Tab",
741 "launcher/switcher/down": "Down",
742 "launcher/switcher/up": "Up",
743- # Panel
744+ # Quicklist:
745+ "quicklist/keynav/first": "Home",
746+ "quicklist/keynav/last": "End",
747+ "quicklist/keynav/next": "Down",
748+ "quicklist/keynav/prev": "Up",
749+ "quicklist/keynav/activate": "Enter",
750+ "quicklist/keynav/exit": "Escape",
751+ # Panel:
752 "panel/show_menus": "Alt",
753 "panel/open_first_menu": ('unityshell', 'panel_first_menu'),
754 "panel/next_indicator": "Right",
755@@ -65,12 +72,12 @@
756 "dash/reveal": "Super",
757 "dash/lens/next": "Ctrl+Tab",
758 "dash/lens/prev": "Ctrl+Shift+Tab",
759- # Lenses
760+ # Lenses:
761 "lens_reveal/command": ("unityshell", "execute_command"),
762 "lens_reveal/apps": "Super+a",
763 "lens_reveal/files": "Super+f",
764 "lens_reveal/music": "Super+m",
765- # Hud
766+ # Hud:
767 "hud/reveal": ("unityshell", "show_hud"),
768 # Switcher:
769 "switcher/reveal_normal": ("unityshell", "alt_tab_forward"),
770@@ -96,7 +103,7 @@
771 "workspace/move_right": ("wall", "right_key"),
772 "workspace/move_up": ("wall", "up_key"),
773 "workspace/move_down": ("wall", "down_key"),
774- # Window management
775+ # Window management:
776 "window/show_desktop" : ("core", "show_desktop_key"),
777 "window/minimize": ("core", "minimize_window_key"),
778 "window/maximize": ("core", "maximize_window_key"),
779
780=== modified file 'tests/autopilot/autopilot/tests/test_quicklist.py'
781--- tests/autopilot/autopilot/tests/test_quicklist.py 2012-04-02 20:18:58 +0000
782+++ tests/autopilot/autopilot/tests/test_quicklist.py 2012-04-19 00:02:18 +0000
783@@ -1,16 +1,19 @@
784 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
785 # Copyright 2012 Canonical
786-# Author: Thomi Richards
787+# Author: Thomi Richards,
788+# Marco Trevisan (Treviño)
789 #
790 # This program is free software: you can redistribute it and/or modify it
791 # under the terms of the GNU General Public License version 3, as published
792 # by the Free Software Foundation.
793
794 import os.path
795-from testtools.matchers import Not, Is, Contains
796+from testtools.matchers import Contains, Equals, NotEquals
797 from xdg.DesktopEntry import DesktopEntry
798+from time import sleep
799
800 from autopilot.emulators.unity.quicklist import QuicklistMenuItemLabel
801+from autopilot.matchers import Eventually
802 from autopilot.tests import AutopilotTestCase
803
804
805@@ -26,22 +29,23 @@
806 self.start_app(self.app_name)
807
808 # load the desktop file from disk:
809- desktop_file = os.path.join('/usr/share/applications',
810- self.KNOWN_APPS[self.app_name]['desktop-file']
811- )
812+ desktop_id = self.KNOWN_APPS[self.app_name]['desktop-file']
813+ desktop_file = os.path.join('/usr/share/applications', desktop_id)
814 de = DesktopEntry(desktop_file)
815 # get the launcher icon from the launcher:
816- launcher_icon = self.launcher.model.get_icon_by_tooltip_text(de.getName())
817- self.assertThat(launcher_icon, Not(Is(None)))
818+ launcher_icon = self.launcher.model.get_icon_by_desktop_id(desktop_id)
819+ self.assertThat(launcher_icon, NotEquals(None))
820
821 # open the icon quicklist, and get all the text labels:
822 launcher = self.launcher.get_launcher_for_monitor(0)
823 launcher.click_launcher_icon(launcher_icon, button=3)
824+ self.addCleanup(self.keyboard.press_and_release, "Escape")
825 ql = launcher_icon.get_quicklist()
826 ql_item_texts = [i.text for i in ql.items if type(i) is QuicklistMenuItemLabel]
827
828 # iterate over all the actions from the desktop file, make sure they're
829 # present in the quicklist texts.
830+ # FIXME, this doesn't work using a locale other than English.
831 actions = de.getActions()
832 for action in actions:
833 key = 'Desktop Action ' + action
834@@ -50,3 +54,197 @@
835 self.assertThat(ql_item_texts, Contains(name))
836
837
838+class QuicklistKeyNavigationTests(AutopilotTestCase):
839+ """Tests for the quicklist key navigation."""
840+
841+ def setUp(self):
842+ super(QuicklistKeyNavigationTests, self).setUp()
843+
844+ self.ql_app = self.start_app("Text Editor")
845+
846+ self.ql_launcher_icon = self.launcher.model.get_icon_by_desktop_id(self.ql_app.desktop_file)
847+ self.assertThat(self.ql_launcher_icon, NotEquals(None))
848+
849+ self.ql_launcher = self.launcher.get_launcher_for_monitor(0)
850+
851+ def open_quicklist_with_mouse(self):
852+ """Opens a quicklist with the mouse."""
853+ self.ql_launcher.click_launcher_icon(self.ql_launcher_icon, button=3)
854+ self.addCleanup(self.keyboard.press_and_release, "Escape")
855+ self.quicklist = self.ql_launcher_icon.get_quicklist()
856+ self.assertThat(self.quicklist, NotEquals(None))
857+ self.quicklist.move_mouse_to_right()
858+ self.assertThat(self.quicklist.selected_item, Equals(None))
859+
860+ def open_quicklist_with_keyboard(self):
861+ """Opens a quicklist using the keyboard."""
862+ self.screen_geo.move_mouse_to_monitor(0)
863+ self.ql_launcher.key_nav_start()
864+ self.addCleanup(self.ql_launcher.key_nav_cancel)
865+
866+ for icon in self.launcher.model.get_launcher_icons():
867+ if icon.tooltip_text != self.ql_app.name:
868+ self.ql_launcher.key_nav_next()
869+ else:
870+ self.keybinding("launcher/keynav/open-quicklist")
871+ self.addCleanup(self.keybinding, "launcher/keynav/close-quicklist")
872+ break
873+
874+ self.quicklist = self.ql_launcher_icon.get_quicklist()
875+ self.assertThat(self.quicklist, NotEquals(None))
876+ self.assertThat(self.quicklist.selected_item, NotEquals(None))
877+
878+ def test_keynav_selects_first_item_when_unselected(self):
879+ """Home key MUST select the first selectable item in a quicklist."""
880+ self.open_quicklist_with_mouse()
881+
882+ self.keybinding("quicklist/keynav/first")
883+
884+ expected_item = self.quicklist.selectable_items[0]
885+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
886+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
887+
888+ def test_keynav_selects_first_item_when_selected(self):
889+ """Home key MUST select the first selectable item in a quicklist when
890+ another item is selected.
891+ """
892+ self.open_quicklist_with_mouse()
893+ mouse_item = self.quicklist.selectable_items[-1]
894+ mouse_item.mouse_move_to()
895+ self.assertThat(mouse_item.selected, Eventually(Equals(True)))
896+
897+ self.keybinding("quicklist/keynav/first")
898+
899+ expected_item = self.quicklist.selectable_items[0]
900+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
901+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
902+
903+ def test_keynav_next_selects_first_item_when_unselected(self):
904+ """Down key MUST select the first valid item when nothing is selected."""
905+ self.open_quicklist_with_mouse()
906+
907+ self.keybinding("quicklist/keynav/next")
908+
909+ expected_item = self.quicklist.selectable_items[0]
910+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
911+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
912+
913+ def test_keynav_selects_last_item_when_unselected(self):
914+ """End key MUST select the last selectable item in a quicklist."""
915+ self.open_quicklist_with_mouse()
916+
917+ self.keybinding("quicklist/keynav/last")
918+
919+ expected_item = self.quicklist.selectable_items[-1]
920+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
921+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
922+
923+ def test_keynav_selects_last_item_when_selected(self):
924+ """End key MUST select the last selectable item in a quicklist when
925+ another item is selected.
926+ """
927+ self.open_quicklist_with_mouse()
928+ mouse_item = self.quicklist.selectable_items[0]
929+ mouse_item.mouse_move_to()
930+ self.assertThat(mouse_item.selected, Eventually(Equals(True)))
931+
932+ self.keybinding("quicklist/keynav/last")
933+
934+ expected_item = self.quicklist.selectable_items[-1]
935+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
936+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
937+
938+ def test_keynav_prev_selects_last_item_when_unselected(self):
939+ """Up key MUST select the last valid item when nothing is selected."""
940+ self.open_quicklist_with_mouse()
941+
942+ self.keybinding("quicklist/keynav/prev")
943+
944+ expected_item = self.quicklist.selectable_items[-1]
945+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
946+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
947+
948+ def test_launcher_keynav_selects_first_item(self):
949+ """The first selectable item of the quicklist must be selected when
950+ opening the quicklist using the launcher key navigation.
951+ """
952+ self.open_quicklist_with_keyboard()
953+
954+ expected_item = self.quicklist.selectable_items[0]
955+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
956+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
957+
958+ def test_keynav_next_selection_works(self):
959+ """Down key MUST select the next valid item."""
960+ self.open_quicklist_with_mouse()
961+
962+ for item in self.quicklist.selectable_items:
963+ self.keybinding("quicklist/keynav/next")
964+ self.assertThat(item.selected, Eventually(Equals(True)))
965+ self.assertThat(self.quicklist.selected_item.id, Equals(item.id))
966+
967+ def test_keynav_prev_selection_works(self):
968+ """Up key MUST select the previous valid item."""
969+ self.open_quicklist_with_mouse()
970+
971+ for item in reversed(self.quicklist.selectable_items):
972+ self.keybinding("quicklist/keynav/prev")
973+ self.assertThat(item.selected, Eventually(Equals(True)))
974+ self.assertThat(self.quicklist.selected_item.id, Equals(item.id))
975+
976+ def test_keynav_prev_is_cyclic(self):
977+ """Up key MUST select the last item, when the first one is selected."""
978+ self.open_quicklist_with_mouse()
979+
980+ mouse_item = self.quicklist.selectable_items[0]
981+ mouse_item.mouse_move_to()
982+ self.assertThat(mouse_item.selected, Eventually(Equals(True)))
983+
984+ self.keybinding("quicklist/keynav/prev")
985+ expected_item = self.quicklist.selectable_items[-1]
986+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
987+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
988+
989+ def test_keynav_next_is_cyclic(self):
990+ """Down key MUST select the first item, when the last one is selected."""
991+ self.open_quicklist_with_mouse()
992+
993+ mouse_item = self.quicklist.selectable_items[-1]
994+ mouse_item.mouse_move_to()
995+ self.assertThat(mouse_item.selected, Eventually(Equals(True)))
996+
997+ self.keybinding("quicklist/keynav/next")
998+ expected_item = self.quicklist.selectable_items[0]
999+ self.assertThat(expected_item.selected, Eventually(Equals(True)))
1000+ self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
1001+
1002+ def test_keynav_mouse_interaction(self):
1003+ """Tests that the interaction between key-navigation and mouse works as
1004+ expected. See bug #911561.
1005+ """
1006+ self.open_quicklist_with_mouse()
1007+ mouse_item = self.quicklist.selectable_items[-1]
1008+ mouse_item.mouse_move_to()
1009+ self.assertThat(mouse_item.selected, Eventually(Equals(True)))
1010+
1011+ self.keybinding("quicklist/keynav/prev")
1012+ sleep(.1)
1013+ self.keybinding("quicklist/keynav/prev")
1014+
1015+ key_item = self.quicklist.selectable_items[-3]
1016+ self.assertThat(key_item.selected, Eventually(Equals(True)))
1017+ self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1018+
1019+ # Moving the mouse horizontally doesn't change the selection
1020+ self.mouse.move(mouse_item.x + mouse_item.width - 10, mouse_item.y + mouse_item.height / 2)
1021+ self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1022+
1023+ # Moving the mouse outside doesn't change the selection
1024+ self.mouse.move(mouse_item.x + mouse_item.width + 50, mouse_item.y + mouse_item.height / 2)
1025+ self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1026+
1027+ # Moving the mouse to another entry, changes the selection
1028+ mouse_item = self.quicklist.selectable_items[-2]
1029+ mouse_item.mouse_move_to()
1030+ self.assertThat(mouse_item.selected, Eventually(Equals(True)))
1031+ self.assertThat(self.quicklist.selected_item.id, Equals(mouse_item.id))
1032
1033=== modified file 'tests/unit/TestQuicklistMenuitems.cpp'
1034--- tests/unit/TestQuicklistMenuitems.cpp 2012-03-14 06:24:18 +0000
1035+++ tests/unit/TestQuicklistMenuitems.cpp 2012-04-19 00:02:18 +0000
1036@@ -94,6 +94,7 @@
1037 g_assert_cmpstr(qlCheckmarkItem->GetLabel(), == , "Unchecked");
1038 g_assert_cmpint(qlCheckmarkItem->GetEnabled(), == , false);
1039 g_assert_cmpint(qlCheckmarkItem->GetActive(), == , false);
1040+ g_assert_cmpint(qlCheckmarkItem->GetSelectable(), == , false);
1041 g_assert_cmpint(qlCheckmarkItem->IsMarkupEnabled(), == , false);
1042
1043 //qlCheckmarkItem->sigChanged.connect (sigc::mem_fun (pointerToCallerClass,
1044@@ -129,6 +130,7 @@
1045
1046 g_assert_cmpstr(qlLabelItem->GetLabel(), == , "A Label");
1047 g_assert_cmpint(qlLabelItem->GetEnabled(), == , true);
1048+ g_assert_cmpint(qlLabelItem->GetSelectable(), == , true);
1049 g_assert_cmpint(qlLabelItem->IsMarkupEnabled(), == , true);
1050
1051 //qlLabelItem->sigChanged.connect (sigc::mem_fun (pointerToCallerClass,
1052@@ -169,6 +171,7 @@
1053 g_assert_cmpstr(qlRadioItem->GetLabel(), == , "Radio Active");
1054 g_assert_cmpint(qlRadioItem->GetEnabled(), == , true);
1055 g_assert_cmpint(qlRadioItem->GetActive(), == , true);
1056+ g_assert_cmpint(qlRadioItem->GetSelectable(), == , true);
1057 g_assert_cmpint(qlRadioItem->IsMarkupEnabled(), == , true);
1058
1059 //qlRadioItem->sigChanged.connect (sigc::mem_fun (pointerToCallerClass,
1060@@ -198,6 +201,7 @@
1061 qlSeparatorItem = new QuicklistMenuItemSeparator(item, true);
1062
1063 g_assert_cmpint(qlSeparatorItem->GetEnabled(), == , true);
1064+ g_assert_cmpint(qlSeparatorItem->GetSelectable(), == , false);
1065
1066 qlSeparatorItem->Dispose();
1067 g_object_unref(item);