Merge lp:~aacid/unity-2d/dash_tab_handling into lp:unity-2d
- dash_tab_handling
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Paweł Stołowski |
Approved revision: | 1061 |
Merged at revision: | 1066 |
Proposed branch: | lp:~aacid/unity-2d/dash_tab_handling |
Merge into: | lp:unity-2d |
Diff against target: |
508 lines (+334/-2) 12 files modified
libunity-2d-private/src/focuspath.cpp (+35/-1) libunity-2d-private/src/focuspath.h (+6/-0) shell/dash/Dash.qml (+32/-0) shell/dash/Filter.qml (+3/-0) shell/dash/FilterCheckoption.qml (+4/-0) shell/dash/FilterLoader.qml (+6/-0) shell/dash/FilterPane.qml (+44/-0) shell/dash/LensView.qml (+24/-0) shell/dash/ListViewWithHeaders.qml (+51/-1) shell/dash/ListViewWithScrollbar.qml (+20/-0) shell/dash/RendererGrid.qml (+8/-0) tests/manual-tests/dash.txt (+101/-0) |
To merge this branch: | bzr merge lp:~aacid/unity-2d/dash_tab_handling |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gerry Boland (community) | Approve | ||
Review via email: mp+101355@code.launchpad.net |
Commit message
[Dash] Handle Tab and Shift+Tab to navigate between the headers
Description of the change
[Dash] Handle Tab and Shift+Tab to navigate between the headers
UNBLOCK
Albert Astals Cid (aacid) wrote : | # |
Paweł Stołowski (stolowski) wrote : | # |
Can we get rid of console.log? I found only 8 occurences of it in our existing code, only for critical issues (well, except for 2 debug statements in VisibilityContr
Albert Astals Cid (aacid) wrote : | # |
Well, the console.log are there to signal a critical problem, if you got there something went very wrong as the focus is in an unexpected place and tab won't work. In my opinion if that ever happens it will help us a lot of the user having the problem can see it.
Gerry Boland (gerboland) wrote : | # |
Tests are correct, code correctly offers behaviours as defined in bug report.
Some UI elements not reacting to focus change, need fix for bug 893966 to resolve that (in progress).
I approve of this change as it resolves a big UX delta with Unity. SRU required before merging however.
Unity Merger (unity-merger) wrote : | # |
Attempt to merge into lp:unity-2d failed due to conflicts:
text conflict in tests/manual-
- 1061. By Albert Astals Cid
-
Merge lp:unity-2d
Preview Diff
1 | === modified file 'libunity-2d-private/src/focuspath.cpp' |
2 | --- libunity-2d-private/src/focuspath.cpp 2012-03-08 19:10:11 +0000 |
3 | +++ libunity-2d-private/src/focuspath.cpp 2012-04-16 13:48:20 +0000 |
4 | @@ -193,8 +193,8 @@ |
5 | (index < m_path.size())) { |
6 | QDeclarativeItem* focus = m_path[index].second; |
7 | Q_ASSERT(focus); |
8 | + m_currentPosition = index; |
9 | focus->setFocus(true); |
10 | - m_currentPosition = index; |
11 | Q_EMIT currentIndexChanged(); |
12 | Q_EMIT currentItemChanged(); |
13 | } |
14 | @@ -254,6 +254,40 @@ |
15 | Q_EMIT currentItemChanged(); |
16 | } |
17 | |
18 | +void FocusPath::focusLastRow() |
19 | +{ |
20 | + updatePosition(((m_path.size() - 1) / m_columns) * m_columns); |
21 | +} |
22 | + |
23 | +bool FocusPath::moveToNext() |
24 | +{ |
25 | + if (m_currentPosition + 1 < m_path.size()) { |
26 | + updatePosition(m_currentPosition + 1); |
27 | + return true; |
28 | + } else { |
29 | + return false; |
30 | + } |
31 | +} |
32 | + |
33 | +bool FocusPath::moveToPrevious() |
34 | +{ |
35 | + if (m_currentPosition - 1 >= 0) { |
36 | + updatePosition(m_currentPosition - 1); |
37 | + return true; |
38 | + } else { |
39 | + return false; |
40 | + } |
41 | +} |
42 | + |
43 | +int FocusPath::previousIndex() |
44 | +{ |
45 | + if (m_currentPosition - 1 >= 0) { |
46 | + return m_path[m_currentPosition - 1].first; |
47 | + } else { |
48 | + return -1; |
49 | + } |
50 | +} |
51 | + |
52 | void FocusPath::addItem(QDeclarativeItem *item) |
53 | { |
54 | if (item->flags() & QGraphicsItem::ItemIsFocusScope) { |
55 | |
56 | === modified file 'libunity-2d-private/src/focuspath.h' |
57 | --- libunity-2d-private/src/focuspath.h 2011-12-19 19:29:14 +0000 |
58 | +++ libunity-2d-private/src/focuspath.h 2012-04-16 13:48:20 +0000 |
59 | @@ -73,6 +73,12 @@ |
60 | QList<PathItem > path() const; |
61 | |
62 | Q_INVOKABLE void reset(); |
63 | + Q_INVOKABLE void focusLastRow(); |
64 | + |
65 | + Q_INVOKABLE bool moveToNext(); |
66 | + Q_INVOKABLE bool moveToPrevious(); |
67 | + |
68 | + Q_INVOKABLE int previousIndex(); |
69 | |
70 | static FocusPathAttached *qmlAttachedProperties(QObject *object); |
71 | |
72 | |
73 | === modified file 'shell/dash/Dash.qml' |
74 | --- shell/dash/Dash.qml 2012-04-10 09:06:40 +0000 |
75 | +++ shell/dash/Dash.qml 2012-04-16 13:48:20 +0000 |
76 | @@ -266,6 +266,38 @@ |
77 | (event.key == Qt.Key_PageUp && event.modifiers == Qt.ControlModifier)) { |
78 | changeLens(-1); |
79 | event.accepted = true |
80 | + } else if (event.key == Qt.Key_Tab && event.modifiers == Qt.NoModifier) { |
81 | + if (search_entry.activeFocus || lensBar.activeFocus || filterPane.activeFocus) { |
82 | + if (!pageLoader.item.isListEmpty()) { |
83 | + pageLoader.item.focusFirstHeader() |
84 | + } else if (filterPane.visible) { |
85 | + filterPane.focusFirstHeader() |
86 | + } |
87 | + } else if (pageLoader.activeFocus) { |
88 | + if (filterPane.visible) { |
89 | + filterPane.focusFirstHeader() |
90 | + } else if (!pageLoader.item.isListEmpty()) { |
91 | + pageLoader.item.focusFirstHeader() |
92 | + } |
93 | + } else { |
94 | + console.log("Dash: Tab pressed with focus in unexpected item") |
95 | + } |
96 | + event.accepted = true |
97 | + } else if (event.key == Qt.Key_Backtab && event.modifiers == Qt.ShiftModifier) { |
98 | + if (filterPane.activeFocus) { |
99 | + if (!pageLoader.item.isListEmpty()) { |
100 | + pageLoader.item.focusLastHeader() |
101 | + } |
102 | + } else if (search_entry.activeFocus || lensBar.activeFocus || pageLoader.activeFocus) { |
103 | + if (filterPane.visible) { |
104 | + filterPane.focusLastHeader() |
105 | + } else if (!pageLoader.item.isListEmpty()) { |
106 | + pageLoader.item.focusLastHeader() |
107 | + } |
108 | + } else { |
109 | + console.log("Dash: Tab pressed with focus in unexpected item") |
110 | + } |
111 | + event.accepted = true |
112 | } |
113 | } |
114 | |
115 | |
116 | === modified file 'shell/dash/Filter.qml' |
117 | --- shell/dash/Filter.qml 2012-03-19 17:26:31 +0000 |
118 | +++ shell/dash/Filter.qml 2012-04-16 13:48:20 +0000 |
119 | @@ -26,4 +26,7 @@ |
120 | property int sectionSpacing: 12 |
121 | |
122 | height: childrenRect.height |
123 | + |
124 | + function focusFirstItem() { |
125 | + } |
126 | } |
127 | |
128 | === modified file 'shell/dash/FilterCheckoption.qml' |
129 | --- shell/dash/FilterCheckoption.qml 2012-03-19 17:26:31 +0000 |
130 | +++ shell/dash/FilterCheckoption.qml 2012-04-16 13:48:20 +0000 |
131 | @@ -25,6 +25,10 @@ |
132 | |
133 | property variant grid_columns : 2 |
134 | |
135 | + function focusFirstItem() { |
136 | + filters.currentIndex = 0 |
137 | + } |
138 | + |
139 | GridViewWithSpacing { |
140 | id: filters |
141 | |
142 | |
143 | === modified file 'shell/dash/FilterLoader.qml' |
144 | --- shell/dash/FilterLoader.qml 2012-01-24 12:14:59 +0000 |
145 | +++ shell/dash/FilterLoader.qml 2012-04-16 13:48:20 +0000 |
146 | @@ -27,6 +27,12 @@ |
147 | property variant lens |
148 | property variant filterModel |
149 | property bool isFirst |
150 | + property alias header: header |
151 | + |
152 | + function focusHeader() { |
153 | + header.focus = true |
154 | + filterView.item.focusFirstItem() |
155 | + } |
156 | |
157 | height: childrenRect.height |
158 | |
159 | |
160 | === modified file 'shell/dash/FilterPane.qml' |
161 | --- shell/dash/FilterPane.qml 2012-03-05 14:52:17 +0000 |
162 | +++ shell/dash/FilterPane.qml 2012-04-16 13:48:20 +0000 |
163 | @@ -30,6 +30,20 @@ |
164 | /* Give the focus to header when folded */ |
165 | onFoldedChanged: if (folded) header.focus = true |
166 | |
167 | + function focusFirstHeader() { |
168 | + header.forceActiveFocus() |
169 | + options.currentIndex = 0 |
170 | + } |
171 | + |
172 | + function focusLastHeader() { |
173 | + if (folded) { |
174 | + focusFirstHeader() |
175 | + } else { |
176 | + options.currentIndex = options.count - 1 |
177 | + options.forceActiveFocus() |
178 | + } |
179 | + } |
180 | + |
181 | AbstractButton { |
182 | id: header |
183 | objectName: "filterResults" |
184 | @@ -88,6 +102,36 @@ |
185 | } |
186 | } |
187 | |
188 | + Keys.onPressed: { |
189 | + if (event.key == Qt.Key_Tab && event.modifiers == Qt.NoModifier) { |
190 | + if (!folded) { |
191 | + if (header.focus) { |
192 | + options.forceActiveFocus() |
193 | + options.currentIndex = 0 |
194 | + event.accepted = true |
195 | + } else if (options.currentIndex + 1 < options.count) { |
196 | + options.currentItem.focusHeader() |
197 | + options.currentIndex++ |
198 | + event.accepted = true |
199 | + } |
200 | + } |
201 | + } else if (event.key == Qt.Key_Backtab && event.modifiers == Qt.ShiftModifier) { |
202 | + if (!folded && !header.focus) { |
203 | + if (options.currentItem.header.activeFocus) { |
204 | + if (options.currentIndex > 0) { |
205 | + options.currentIndex-- |
206 | + options.currentItem.focusHeader() |
207 | + } else { |
208 | + header.forceActiveFocus() |
209 | + } |
210 | + } else { |
211 | + options.currentItem.focusHeader() |
212 | + } |
213 | + event.accepted = true |
214 | + } |
215 | + } |
216 | + } |
217 | + |
218 | ListView { |
219 | id: options |
220 | |
221 | |
222 | === modified file 'shell/dash/LensView.qml' |
223 | --- shell/dash/LensView.qml 2012-03-30 14:56:08 +0000 |
224 | +++ shell/dash/LensView.qml 2012-04-16 13:48:20 +0000 |
225 | @@ -28,6 +28,18 @@ |
226 | property variant model |
227 | property string firstNonEmptyCategory |
228 | |
229 | + function focusFirstHeader() { |
230 | + results.focusFirstHeader() |
231 | + } |
232 | + |
233 | + function focusLastHeader() { |
234 | + results.focusLastHeader() |
235 | + } |
236 | + |
237 | + function isListEmpty() { |
238 | + return results.isListEmpty() |
239 | + } |
240 | + |
241 | function updateFirstCategory() { |
242 | if (lensView.model.results.count == 0) |
243 | return |
244 | @@ -108,6 +120,18 @@ |
245 | } |
246 | } |
247 | |
248 | + Keys.onPressed: { |
249 | + if (event.key == Qt.Key_Tab && event.modifiers == Qt.NoModifier) { |
250 | + if (results.focusNextHeader()) { |
251 | + event.accepted = true |
252 | + } |
253 | + } else if (event.key == Qt.Key_Backtab && event.modifiers == Qt.ShiftModifier) { |
254 | + if (results.focusPreviousHeader()) { |
255 | + event.accepted = true |
256 | + } |
257 | + } |
258 | + } |
259 | + |
260 | ListViewWithScrollbar { |
261 | id: results |
262 | |
263 | |
264 | === modified file 'shell/dash/ListViewWithHeaders.qml' |
265 | --- shell/dash/ListViewWithHeaders.qml 2011-11-23 10:27:05 +0000 |
266 | +++ shell/dash/ListViewWithHeaders.qml 2012-04-16 13:48:20 +0000 |
267 | @@ -43,6 +43,46 @@ |
268 | |
269 | property bool giveFocus: false |
270 | |
271 | + function focusFirstHeader() { |
272 | + focusPath.reset() |
273 | + focusPath.currentItem.header.forceActiveFocus() |
274 | + } |
275 | + |
276 | + function focusLastHeader() { |
277 | + focusPath.focusLastRow() |
278 | + focusPath.currentItem.header.forceActiveFocus() |
279 | + } |
280 | + |
281 | + function focusNextHeader() { |
282 | + var moved = focusPath.moveToNext() |
283 | + if (moved) { |
284 | + focusPath.currentItem.header.forceActiveFocus() |
285 | + } |
286 | + return moved |
287 | + } |
288 | + |
289 | + function focusPreviousHeader() { |
290 | + if (!focusPath.currentItem.header.activeFocus) { |
291 | + focusPath.currentItem.header.forceActiveFocus() |
292 | + return true |
293 | + } else { |
294 | + var moved = focusPath.moveToPrevious() |
295 | + if (moved) { |
296 | + focusPath.currentItem.header.forceActiveFocus() |
297 | + } |
298 | + return moved |
299 | + } |
300 | + } |
301 | + |
302 | + function isListEmpty() { |
303 | + var empty = true |
304 | + for (var i = 0; empty && i < categories.count; i++) { |
305 | + var category = categories.itemAt(i); |
306 | + empty = !category.body.visible && !category.header.visible |
307 | + } |
308 | + return empty |
309 | + } |
310 | + |
311 | FocusPath { |
312 | id: focusPath |
313 | item: categoriesColumn |
314 | @@ -75,7 +115,7 @@ |
315 | var newContentY = -1; |
316 | |
317 | if (scroll.contentY > itemTop) { |
318 | - newContentY = itemTop |
319 | + newContentY = Math.max(0, itemTop) |
320 | } else if ((scroll.contentY + scroll.height) < itemBottom) { |
321 | newContentY = itemBottom - scroll.height; |
322 | } |
323 | @@ -101,6 +141,9 @@ |
324 | FocusPath.index: index |
325 | FocusPath.skip: !headerLoader.item.visible && !bodyLoader.item.visible |
326 | |
327 | + property alias bodyLoader: bodyLoader |
328 | + property alias body: bodyLoader.item |
329 | + property alias header: headerLoader.item |
330 | |
331 | Column { |
332 | |
333 | @@ -125,6 +168,13 @@ |
334 | |
335 | onActiveFocusChanged: { |
336 | if (visible && item && item.activeFocus) { |
337 | + var categoryOnTopIndex = focusPath.previousIndex() |
338 | + if (categoryOnTopIndex != -1) { |
339 | + categories.itemAt(categoryOnTopIndex).body.item.focusLastRow() |
340 | + categories.itemAt(categoryOnTopIndex).bodyLoader.focus = true |
341 | + } |
342 | + |
343 | + focusPath.currentItem.body.item.focusFirstElement() |
344 | scroll.moveToPosition(item) |
345 | } |
346 | } |
347 | |
348 | === modified file 'shell/dash/ListViewWithScrollbar.qml' |
349 | --- shell/dash/ListViewWithScrollbar.qml 2011-12-07 14:57:53 +0000 |
350 | +++ shell/dash/ListViewWithScrollbar.qml 2012-04-16 13:48:20 +0000 |
351 | @@ -25,6 +25,26 @@ |
352 | property alias bodyDelegate: list.bodyDelegate |
353 | property alias headerDelegate: list.headerDelegate |
354 | |
355 | + function focusFirstHeader() { |
356 | + list.focusFirstHeader() |
357 | + } |
358 | + |
359 | + function focusLastHeader() { |
360 | + list.focusLastHeader() |
361 | + } |
362 | + |
363 | + function focusNextHeader() { |
364 | + return list.focusNextHeader() |
365 | + } |
366 | + |
367 | + function focusPreviousHeader() { |
368 | + return list.focusPreviousHeader() |
369 | + } |
370 | + |
371 | + function isListEmpty() { |
372 | + return list.isListEmpty() |
373 | + } |
374 | + |
375 | ListViewWithHeaders { |
376 | id: list |
377 | |
378 | |
379 | === modified file 'shell/dash/RendererGrid.qml' |
380 | --- shell/dash/RendererGrid.qml 2012-02-10 20:37:18 +0000 |
381 | +++ shell/dash/RendererGrid.qml 2012-04-16 13:48:20 +0000 |
382 | @@ -43,6 +43,14 @@ |
383 | |
384 | property bool centered: true |
385 | |
386 | + function focusFirstElement() { |
387 | + focusPath.reset() |
388 | + } |
389 | + |
390 | + function focusLastRow() { |
391 | + focusPath.focusLastRow() |
392 | + } |
393 | + |
394 | FocusPath { |
395 | id: focusPath |
396 | |
397 | |
398 | === modified file 'tests/manual-tests/dash.txt' |
399 | --- tests/manual-tests/dash.txt 2012-04-11 13:46:44 +0000 |
400 | +++ tests/manual-tests/dash.txt 2012-04-16 13:48:20 +0000 |
401 | @@ -127,6 +127,107 @@ |
402 | ----- |
403 | |
404 | # Test case objectives: |
405 | + # * Check that Tab moves between headers on the dash |
406 | + # Pre-conditions |
407 | + # * Have a files and folders lens with 3 categories |
408 | + # Test steps |
409 | + # * Open dash |
410 | + # * Switch to the files and folders lens |
411 | + # * Verify focus is on the search entry |
412 | + # * Press Tab |
413 | + # * Verify focus is on the first category header |
414 | + # * Press Tab |
415 | + # * Verify focus is on the second category header |
416 | + # * Press Tab |
417 | + # * Verify focus is on the third category header |
418 | + # * Press Tab |
419 | + |
420 | +----- |
421 | + |
422 | + # Test case objectives: |
423 | + # * Check that Tab moves between headers and Filters on the dash |
424 | + # Pre-conditions |
425 | + # * Have a files and folders lens with 3 categories |
426 | + # * Have the focus on the third header of files and folders lens |
427 | + # * Have the filters collapsed |
428 | + # Test steps |
429 | + # * Verify focus is on the "Filter results" text |
430 | + # * Press Tab |
431 | + # * Verify focus is on the first category header |
432 | + |
433 | +----- |
434 | + |
435 | + # Test case objectives: |
436 | + # * Check that Tab moves between Filters "All" buttons |
437 | + # Pre-conditions |
438 | + # * Be on the files and folders lens |
439 | + # * Have the focus on the "Filter results" text |
440 | + # * Have the filters collapsed |
441 | + # Test steps |
442 | + # * Verify focus is on the "Filter results" text |
443 | + # * Press Enter to expand the filters |
444 | + # * Press Tab |
445 | + # * Verify focus is on the "Last Modified" filter "All" button |
446 | + # * Press Tab |
447 | + # * Verify focus is on the "Type" filter "All" button |
448 | + # * Press Tab |
449 | + # * Verify focus is on the "Size" filter "All" button |
450 | + # * Press Tab |
451 | + # * Verify focus is on the first category header |
452 | + |
453 | +----- |
454 | + |
455 | + # Test case objectives: |
456 | + # * Check that Tab goes to the next category header when not on the header |
457 | + # Pre-conditions |
458 | + # * Be on the files and folders lens |
459 | + # * Have the focus on the first category header |
460 | + # Test steps |
461 | + # * Press Down Arrow |
462 | + # * Verify focus is on the first element of the first category |
463 | + # * Press Tab |
464 | + # * Verify focus is on the second category header |
465 | + |
466 | +----- |
467 | + |
468 | + # Test case objectives: |
469 | + # * Check that Shift+Tab goes to the current category header |
470 | + # Pre-conditions |
471 | + # * Be on the files and folders lens |
472 | + # * Have the focus on the first category header |
473 | + # Test steps |
474 | + # * Press Down Arrow |
475 | + # * Verify focus is on the first element of the first category |
476 | + # * Press Shift+Tab |
477 | + # * Verify focus is on the first category header |
478 | + |
479 | +----- |
480 | + |
481 | + # Test case objectives: |
482 | + # * Check that Shift+Tab cycles through category headers and filters |
483 | + # Pre-conditions |
484 | + # * Be on the files and folders lens |
485 | + # * Have the filters uncollapsed |
486 | + # * Have the focus on the third category header |
487 | + # Test steps |
488 | + # * Press Shift+Tab |
489 | + # * Verify focus is on the second category header |
490 | + # * Press Shift+Tab |
491 | + # * Verify focus is on the first category header |
492 | + # * Press Shift+Tab |
493 | + # * Verify focus is on the "Size" filter "All" button |
494 | + # * Press Shift+Tab |
495 | + # * Verify focus is on the "Type" filter "All" button |
496 | + # * Press Shift+Tab |
497 | + # * Verify focus is on the "Last Modified" filter "All" button |
498 | + # * Press Shift+Tab |
499 | + # * Verify focus is on the "Filter results" text |
500 | + # * Press Shift+Tab |
501 | + # * Verify focus is on the third category header |
502 | + |
503 | +----- |
504 | + |
505 | +# Test case objectives: |
506 | # * Verify the 4 finger tap gesture toggles the dash |
507 | # Pre-conditions |
508 | # * None |
Still WIP (thought it mostly works) and missing tests but I'd like to know if you think this is the way to go