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

Subscribers

People subscribed via source and target branches