Merge lp:~stolowski/unity-2d/hud-highlight-fix into lp:unity-2d

Proposed by Paweł Stołowski on 2012-03-28
Status: Merged
Approved by: Albert Astals Cid on 2012-04-04
Approved revision: 1024
Merged at revision: 1039
Proposed branch: lp:~stolowski/unity-2d/hud-highlight-fix
Merge into: lp:unity-2d
Diff against target: 341 lines (+289/-4)
4 files modified
shell/common/AbstractButton.qml (+4/-2)
shell/hud/Hud.qml (+7/-0)
shell/hud/ResultItem.qml (+2/-2)
tests/hud/hud-search-tests.rb (+276/-0)
To merge this branch: bzr merge lp:~stolowski/unity-2d/hud-highlight-fix
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) 2012-04-03 Approve on 2012-04-04
Gerry Boland 2012-03-28 Pending
Review via email: mp+99756@code.launchpad.net

Commit message

[hud] Highlight one item only in HUD results.

Moving mouse over an item sets currentIndex in the model, rather than relying on "selected" or "hovered" state.
This makes it impossible to have different items highlighted with keyboard and mouse.

Description of the change

[hud] Highlight one item only in HUD results.
Moving mouse over an item sets currentIndex in the model, rather than relying on "selected" or "hovered" state.
This makes it impossible to have different items highlighted with keyboard and mouse.

To post a comment you must log in.
Albert Astals Cid (aacid) wrote :

The tests fail here

  1) Failure:
test_Highlight_should_follow_mouse(HUD_Search_tests)
    [/usr/lib/ruby/vendor_ruby/tdriver/verify/verify.rb:715:in `verify_equal'
     hud-search-tests.rb:46:in `item_is_highlighted'
     hud-search-tests.rb:220:in `test_Highlight_should_follow_mouse'
     /home/tsdgeos_work/unity-2d/unity-2d_trunk/tests/misc/lib/testhelper.rb:114:in `__send__'
     /home/tsdgeos_work/unity-2d/unity-2d_trunk/tests/misc/lib/testhelper.rb:114:in `run'
     /home/tsdgeos_work/unity-2d/unity-2d_trunk/tests/misc/lib/testhelper.rb:39:in `run']:
Verification "Result item #2 should have focus" at hud-search-tests.rb:46:in `item_is_highlighted' failed:
=> verify_equal('true', TIMEOUT, "Result item ##{index+1} should have focus") {
           results[index]['activeFocus']
       }
The value was not equal to true. It returned: "false"

  2) Failure:
test_Mixing_Keyboard___Mouse_navigation_should_highlight_only_one_item(HUD_Search_tests)
    [/usr/lib/ruby/vendor_ruby/tdriver/verify/verify.rb:715:in `verify_equal'
     hud-search-tests.rb:46:in `item_is_highlighted'
     hud-search-tests.rb:264:in `test_Mixing_Keyboard___Mouse_navigation_should_highlight_only_one_item'
     /home/tsdgeos_work/unity-2d/unity-2d_trunk/tests/misc/lib/testhelper.rb:114:in `__send__'
     /home/tsdgeos_work/unity-2d/unity-2d_trunk/tests/misc/lib/testhelper.rb:114:in `run'
     /home/tsdgeos_work/unity-2d/unity-2d_trunk/tests/misc/lib/testhelper.rb:39:in `run']:
Verification "Result item #2 should have focus" at hud-search-tests.rb:46:in `item_is_highlighted' failed:
=> verify_equal('true', TIMEOUT, "Result item ##{index+1} should have focus") {
           results[index]['activeFocus']
       }
The value was not equal to true. It returned: "false"

1022. By Paweł Stołowski on 2012-04-03

Fix testability test - use global Y as starting results postion rather then local Y.

Albert Astals Cid (aacid) wrote :

Verify that exactly 5 result items are displayed
should be 6

review: Needs Fixing
1023. By Paweł Stołowski on 2012-04-03

Fixed test comment.

1024. By Paweł Stołowski on 2012-04-03

Don't reference model.index in ResultItem. Removed hasmouse property and expose mouseOver in AbstractButton, which
is enough.

Albert Astals Cid (aacid) wrote :

Looks good. The forceActiveFocus seems a bit scary, but hopefully we can remove it when fixing 966180

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'shell/common/AbstractButton.qml'
2--- shell/common/AbstractButton.qml 2011-11-29 17:45:27 +0000
3+++ shell/common/AbstractButton.qml 2012-04-03 16:34:19 +0000
4@@ -28,6 +28,8 @@
5 (see http://bugreports.qt.nokia.com/browse/QTBUG-13007). */
6 property bool pressed: false
7
8+ property alias mouseOver: mouse_area.containsMouse
9+
10 signal clicked
11
12 Accessible.role: Accessible.PushButton
13@@ -56,10 +58,10 @@
14 state: {
15 if(pressed || mouse_area.pressed)
16 return "pressed"
17+ else if(activeFocus)
18+ return "selected"
19 else if(mouse_area.containsMouse)
20 return "hovered"
21- else if(activeFocus)
22- return "selected"
23 else
24 return "default"
25 }
26
27=== modified file 'shell/hud/Hud.qml'
28--- shell/hud/Hud.qml 2012-04-02 09:11:31 +0000
29+++ shell/hud/Hud.qml 2012-04-03 16:34:19 +0000
30@@ -235,6 +235,13 @@
31 icon: iconName /* expose this property for tile */
32
33 onClicked: executeResult(resultId)
34+ onMouseOverChanged: {
35+ if (mouseOver) {
36+ resultList.currentIndex = model.index;
37+ // workaround for loosing highlight for mouse if search entry steals focus - see https://bugs.launchpad.net/unity-2d/+bug/966180
38+ forceActiveFocus();
39+ }
40+ }
41 }
42
43 onCurrentItemChanged: {
44
45=== modified file 'shell/hud/ResultItem.qml'
46--- shell/hud/ResultItem.qml 2012-03-06 14:40:24 +0000
47+++ shell/hud/ResultItem.qml 2012-04-03 16:34:19 +0000
48@@ -47,10 +47,10 @@
49 anchors.bottom: parent.bottom
50
51 border.color: "white"
52- border.width: (parent.state == "selected" || parent.state == "hovered"
53+ border.width: (parent.state == "selected"
54 || parent.state == "pressed") ? 2 : 0
55 color: {
56- if (parent.state == "selected" || parent.state == "hovered") return "#21ffffff"
57+ if (parent.state == "selected") return "#21ffffff"
58 else if ( parent.state == "pressed" ) return "white"
59 else return "transparent"
60 }
61
62=== added file 'tests/hud/hud-search-tests.rb'
63--- tests/hud/hud-search-tests.rb 1970-01-01 00:00:00 +0000
64+++ tests/hud/hud-search-tests.rb 2012-04-03 16:34:19 +0000
65@@ -0,0 +1,276 @@
66+#!/usr/bin/env ruby1.8
67+=begin
68+/*
69+ * This file is part of unity-2d
70+ *
71+ * Copyright 2012 Canonical Ltd.
72+ *
73+ * Authors:
74+ * - Pawel Stolowski <pawel.stolowski@canonical.com>
75+ *
76+ * This program is free software; you can redistribute it and/or modify
77+ * it under the terms of the GNU General Public License as published by
78+ * the Free Software Foundation; version 3.
79+ *
80+ * This program is distributed in the hope that it will be useful,
81+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
82+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
83+ * GNU General Public License for more details.
84+ *
85+ * You should have received a copy of the GNU General Public License
86+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
87+ */
88+=end
89+
90+require '../run-tests.rb' unless $INIT_COMPLETED
91+require 'xdo/xwindow'
92+require 'xdo/keyboard'
93+require 'xdo/mouse'
94+
95+HUD_HIGHLIGHT_COLOR = '#ffffff'
96+HUD_NO_HIGHLIGHT_COLOR = '#000000'
97+HUD_RESULTS_START_Y = 77
98+HUD_RESULT_ITEM_HEIGHT = 42
99+
100+######################### Helper functions ############################
101+
102+#
103+# Returns array of ResultItem objects
104+def get_search_results(shell)
105+ return shell.Hud().QDeclarativeItem().QDeclarativeRectangle().QDeclarativeListView().QDeclarativeItem().children({:type=>:ResultItem}, false)
106+end
107+
108+#
109+# Verify that given ResultItem is highlighted and active.
110+def item_is_highlighted(results, index)
111+ verify_equal('true', TIMEOUT, "Result item ##{index+1} should have focus") {
112+ results[index]['activeFocus']
113+ }
114+
115+ verify_equal(HUD_HIGHLIGHT_COLOR, TIMEOUT, "Result item ##{index+1} should be highlighted") {
116+ results[index].QDeclarativeRectangle(:name=>'container')['color']
117+ }
118+end
119+
120+#
121+# Verify that exactly one item is highlighted and active.
122+def only_one_item_highlighted(results)
123+ verify_equal(1, TIMEOUT, 'Exactly one item should be highlighted') {
124+ results.count {|x| x.QDeclarativeRectangle(:name=>'container')['color'] == HUD_HIGHLIGHT_COLOR}
125+ }
126+
127+ verify_equal(1, 0, 'Exactly one item has focus') {
128+ results.count {|x| x['activeFocus'] == 'true'}
129+ }
130+
131+ verify_equal(1, 0, "Exactly one item has 'selected' state ") {
132+ results.count {|x| x['state'] == 'selected'}
133+ }
134+end
135+
136+############################# Test Suite #############################
137+context "HUD Search tests" do
138+
139+ # Run once at the beginning of this test suite
140+ startup do
141+ $SUT.execute_shell_command 'killall unity-2d-shell'
142+ $SUT.execute_shell_command 'killall unity-2d-shell'
143+ end
144+
145+ # Run once at the end of this test suite
146+ shutdown do
147+ end
148+
149+ # Run before each test case begins
150+ setup do
151+ # Execute the application
152+ @app = $SUT.run( :name => UNITY_2D_SHELL,
153+ :arguments => "-testability",
154+ :sleeptime => 2 )
155+ # Make certain application is ready for testing
156+ verify{ @app.LauncherLoader() }
157+
158+ # Move mouse out of way
159+ XDo::Mouse.move(500, 500, 0, true)
160+ end
161+
162+ # Run after each test case completes
163+ teardown do
164+ #Need to kill Shell as it does not shutdown when politely asked
165+ $SUT.execute_shell_command 'pkill -nf unity-2d-shell'
166+ end
167+
168+ #####################################################################################
169+ # Test cases
170+
171+ # Test case objectives:
172+ # * Check that first item is highlighted by default in HUD search results
173+ # Pre-conditions
174+ # * None
175+ # Test steps
176+ # * Tap Alt to open HUD
177+ # * Check that HUD is visible
178+ # * Type search string (letter 'a')
179+ # * Verify that exactly 6 result items are displayed
180+ # * Verify that first result item is highlighted and has focus
181+ # * Verify that exactly one result item is highlighted
182+ # Post-conditions
183+ # * None
184+ # References
185+ # * https://bugs.launchpad.net/unity-2d/+bug/948441
186+ test "First result should be highlighted by default" do
187+ # wait for hud to become ready
188+ sleep(1);
189+
190+ XDo::Keyboard.alt
191+
192+ verify_equal('true', TIMEOUT, 'HUD should be visible') {
193+ @app.Hud()['active']
194+ }
195+
196+ # type search string
197+ XDo::Keyboard.a
198+
199+ results = get_search_results(@app)
200+
201+ verify_equal(6, 0, 'Should get exactly 6 rows') {
202+ results.length
203+ }
204+
205+ item_is_highlighted(results, 0)
206+ only_one_item_highlighted(results)
207+ end
208+
209+ # Test case objectives:
210+ # * Check that keyboard arrows change highlighted item in HUD search results
211+ # Pre-conditions
212+ # * None
213+ # Test steps
214+ # * Tap Alt to open HUD
215+ # * Check that HUD is visible
216+ # * Type search string (letter 'a')
217+ # * Press 'down'
218+ # * Verify that second result item is highlighted and has focus
219+ # * Verify that exactly one result item is highlighted
220+ # Post-conditions
221+ # * None
222+ # References
223+ # * https://bugs.launchpad.net/unity-2d/+bug/948441
224+ test "Highlight should follow keyboard" do
225+ # wait for hud to become ready
226+ sleep(1);
227+
228+ XDo::Keyboard.alt
229+
230+ verify_equal('true', TIMEOUT, 'HUD should be visible') {
231+ @app.Hud()['active']
232+ }
233+
234+ # type search string
235+ XDo::Keyboard.a
236+
237+ verify(TIMEOUT, 'Results list should be visible') {
238+ get_search_results(@app)
239+ }
240+
241+ XDo::Keyboard.down
242+
243+ results = get_search_results(@app)
244+ item_is_highlighted(results, 1)
245+ only_one_item_highlighted(results)
246+ end
247+
248+ # Test case objectives:
249+ # * Check that moving mouse over HUD search results moves highlight
250+ # Pre-conditions
251+ # * None
252+ # Test steps
253+ # * Tap Alt to open HUD
254+ # * Check that HUD is visible
255+ # * Type search string (letter 'a')
256+ # * Move mouse over second results item
257+ # * Verify that second result item is highlighted and has focus
258+ # * Verify that exactly one result item is highlighted
259+ # Post-conditions
260+ # * None
261+ # References
262+ # * https://bugs.launchpad.net/unity-2d/+bug/948441
263+ test "Highlight should follow mouse" do
264+ # wait for hud to become ready
265+ sleep(1);
266+
267+ XDo::Keyboard.alt
268+
269+ verify_equal('true', TIMEOUT, 'HUD should be visible') {
270+ @app.Hud()['active']
271+ }
272+
273+ # type search string
274+ XDo::Keyboard.a
275+
276+ verify(TIMEOUT, 'Results list should be visible') {
277+ get_search_results(@app)
278+ }
279+
280+ # Move mouse over second item
281+ XDo::Mouse.move(100, HUD_RESULTS_START_Y + HUD_RESULT_ITEM_HEIGHT+ 2, 0, true)
282+
283+ results = get_search_results(@app)
284+
285+ item_is_highlighted(results, 1)
286+ only_one_item_highlighted(results)
287+ end
288+
289+ # Test case objectives:
290+ # * Check that moving mouse over 2nd item and then pressing 'Down' key moves highlight to 3rd item
291+ # Pre-conditions
292+ # * None
293+ # Test steps
294+ # * Tap Alt to open HUD
295+ # * Check that HUD is visible
296+ # * Type search string (letter 'a')
297+ # * Move mouse over second results item
298+ # * Verify that second result item is highlighted and has focus
299+ # * Verify that exactly one result item is highlighted
300+ # * Press 'down'
301+ # * Verify that second result item is highlighted and has focus
302+ # * Verify that exactly one result item is highlighted
303+ # Post-conditions
304+ # * None
305+ # References
306+ # * https://bugs.launchpad.net/unity-2d/+bug/948441
307+ test "Mixing Keyboard & Mouse navigation should highlight only one item" do
308+ # wait for hud to become ready
309+ sleep(1);
310+
311+ XDo::Keyboard.alt
312+
313+ verify_equal('true', TIMEOUT, 'HUD should be visible') {
314+ @app.Hud()['active']
315+ }
316+
317+ # type search string
318+ XDo::Keyboard.a
319+
320+ verify(TIMEOUT, 'Results list should be visible') {
321+ get_search_results(@app)
322+ }
323+
324+ # Move mouse over second item
325+ XDo::Mouse.move(100, HUD_RESULTS_START_Y + HUD_RESULT_ITEM_HEIGHT+ 2, 0, true)
326+
327+ results = get_search_results(@app)
328+
329+ item_is_highlighted(results, 1)
330+ only_one_item_highlighted(results)
331+
332+ verify(TIMEOUT, 'Results list should be visible') {
333+ get_search_results(@app)
334+ }
335+
336+ XDo::Keyboard.down
337+
338+ item_is_highlighted(results, 2)
339+ only_one_item_highlighted(results)
340+ end
341+end

Subscribers

People subscribed via source and target branches