Merge lp:~aacid/unity-2d/dash_tab_handling into lp:unity-2d

Proposed by Albert Astals Cid
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
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

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote :

Still WIP (thought it mostly works) and missing tests but I'd like to know if you think this is the way to go

Revision history for this message
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 VisibilityController.qml ;)).

Revision history for this message
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.

Revision history for this message
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.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

Attempt to merge into lp:unity-2d failed due to conflicts:

text conflict in tests/manual-tests/dash.txt

lp:~aacid/unity-2d/dash_tab_handling updated
1061. By Albert Astals Cid

Merge lp:unity-2d

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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

Subscribers

People subscribed via source and target branches