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
=== modified file 'libunity-2d-private/src/focuspath.cpp'
--- libunity-2d-private/src/focuspath.cpp 2012-03-08 19:10:11 +0000
+++ libunity-2d-private/src/focuspath.cpp 2012-04-16 13:48:20 +0000
@@ -193,8 +193,8 @@
193 (index < m_path.size())) {193 (index < m_path.size())) {
194 QDeclarativeItem* focus = m_path[index].second;194 QDeclarativeItem* focus = m_path[index].second;
195 Q_ASSERT(focus);195 Q_ASSERT(focus);
196 m_currentPosition = index;
196 focus->setFocus(true);197 focus->setFocus(true);
197 m_currentPosition = index;
198 Q_EMIT currentIndexChanged();198 Q_EMIT currentIndexChanged();
199 Q_EMIT currentItemChanged();199 Q_EMIT currentItemChanged();
200 }200 }
@@ -254,6 +254,40 @@
254 Q_EMIT currentItemChanged();254 Q_EMIT currentItemChanged();
255}255}
256256
257void FocusPath::focusLastRow()
258{
259 updatePosition(((m_path.size() - 1) / m_columns) * m_columns);
260}
261
262bool FocusPath::moveToNext()
263{
264 if (m_currentPosition + 1 < m_path.size()) {
265 updatePosition(m_currentPosition + 1);
266 return true;
267 } else {
268 return false;
269 }
270}
271
272bool FocusPath::moveToPrevious()
273{
274 if (m_currentPosition - 1 >= 0) {
275 updatePosition(m_currentPosition - 1);
276 return true;
277 } else {
278 return false;
279 }
280}
281
282int FocusPath::previousIndex()
283{
284 if (m_currentPosition - 1 >= 0) {
285 return m_path[m_currentPosition - 1].first;
286 } else {
287 return -1;
288 }
289}
290
257void FocusPath::addItem(QDeclarativeItem *item)291void FocusPath::addItem(QDeclarativeItem *item)
258{292{
259 if (item->flags() & QGraphicsItem::ItemIsFocusScope) {293 if (item->flags() & QGraphicsItem::ItemIsFocusScope) {
260294
=== modified file 'libunity-2d-private/src/focuspath.h'
--- libunity-2d-private/src/focuspath.h 2011-12-19 19:29:14 +0000
+++ libunity-2d-private/src/focuspath.h 2012-04-16 13:48:20 +0000
@@ -73,6 +73,12 @@
73 QList<PathItem > path() const;73 QList<PathItem > path() const;
7474
75 Q_INVOKABLE void reset();75 Q_INVOKABLE void reset();
76 Q_INVOKABLE void focusLastRow();
77
78 Q_INVOKABLE bool moveToNext();
79 Q_INVOKABLE bool moveToPrevious();
80
81 Q_INVOKABLE int previousIndex();
7682
77 static FocusPathAttached *qmlAttachedProperties(QObject *object);83 static FocusPathAttached *qmlAttachedProperties(QObject *object);
7884
7985
=== modified file 'shell/dash/Dash.qml'
--- shell/dash/Dash.qml 2012-04-10 09:06:40 +0000
+++ shell/dash/Dash.qml 2012-04-16 13:48:20 +0000
@@ -266,6 +266,38 @@
266 (event.key == Qt.Key_PageUp && event.modifiers == Qt.ControlModifier)) {266 (event.key == Qt.Key_PageUp && event.modifiers == Qt.ControlModifier)) {
267 changeLens(-1);267 changeLens(-1);
268 event.accepted = true268 event.accepted = true
269 } else if (event.key == Qt.Key_Tab && event.modifiers == Qt.NoModifier) {
270 if (search_entry.activeFocus || lensBar.activeFocus || filterPane.activeFocus) {
271 if (!pageLoader.item.isListEmpty()) {
272 pageLoader.item.focusFirstHeader()
273 } else if (filterPane.visible) {
274 filterPane.focusFirstHeader()
275 }
276 } else if (pageLoader.activeFocus) {
277 if (filterPane.visible) {
278 filterPane.focusFirstHeader()
279 } else if (!pageLoader.item.isListEmpty()) {
280 pageLoader.item.focusFirstHeader()
281 }
282 } else {
283 console.log("Dash: Tab pressed with focus in unexpected item")
284 }
285 event.accepted = true
286 } else if (event.key == Qt.Key_Backtab && event.modifiers == Qt.ShiftModifier) {
287 if (filterPane.activeFocus) {
288 if (!pageLoader.item.isListEmpty()) {
289 pageLoader.item.focusLastHeader()
290 }
291 } else if (search_entry.activeFocus || lensBar.activeFocus || pageLoader.activeFocus) {
292 if (filterPane.visible) {
293 filterPane.focusLastHeader()
294 } else if (!pageLoader.item.isListEmpty()) {
295 pageLoader.item.focusLastHeader()
296 }
297 } else {
298 console.log("Dash: Tab pressed with focus in unexpected item")
299 }
300 event.accepted = true
269 }301 }
270 }302 }
271303
272304
=== modified file 'shell/dash/Filter.qml'
--- shell/dash/Filter.qml 2012-03-19 17:26:31 +0000
+++ shell/dash/Filter.qml 2012-04-16 13:48:20 +0000
@@ -26,4 +26,7 @@
26 property int sectionSpacing: 1226 property int sectionSpacing: 12
2727
28 height: childrenRect.height28 height: childrenRect.height
29
30 function focusFirstItem() {
31 }
29}32}
3033
=== modified file 'shell/dash/FilterCheckoption.qml'
--- shell/dash/FilterCheckoption.qml 2012-03-19 17:26:31 +0000
+++ shell/dash/FilterCheckoption.qml 2012-04-16 13:48:20 +0000
@@ -25,6 +25,10 @@
2525
26 property variant grid_columns : 226 property variant grid_columns : 2
2727
28 function focusFirstItem() {
29 filters.currentIndex = 0
30 }
31
28 GridViewWithSpacing {32 GridViewWithSpacing {
29 id: filters33 id: filters
3034
3135
=== modified file 'shell/dash/FilterLoader.qml'
--- shell/dash/FilterLoader.qml 2012-01-24 12:14:59 +0000
+++ shell/dash/FilterLoader.qml 2012-04-16 13:48:20 +0000
@@ -27,6 +27,12 @@
27 property variant lens27 property variant lens
28 property variant filterModel28 property variant filterModel
29 property bool isFirst29 property bool isFirst
30 property alias header: header
31
32 function focusHeader() {
33 header.focus = true
34 filterView.item.focusFirstItem()
35 }
3036
31 height: childrenRect.height37 height: childrenRect.height
3238
3339
=== modified file 'shell/dash/FilterPane.qml'
--- shell/dash/FilterPane.qml 2012-03-05 14:52:17 +0000
+++ shell/dash/FilterPane.qml 2012-04-16 13:48:20 +0000
@@ -30,6 +30,20 @@
30 /* Give the focus to header when folded */30 /* Give the focus to header when folded */
31 onFoldedChanged: if (folded) header.focus = true31 onFoldedChanged: if (folded) header.focus = true
3232
33 function focusFirstHeader() {
34 header.forceActiveFocus()
35 options.currentIndex = 0
36 }
37
38 function focusLastHeader() {
39 if (folded) {
40 focusFirstHeader()
41 } else {
42 options.currentIndex = options.count - 1
43 options.forceActiveFocus()
44 }
45 }
46
33 AbstractButton {47 AbstractButton {
34 id: header48 id: header
35 objectName: "filterResults"49 objectName: "filterResults"
@@ -88,6 +102,36 @@
88 }102 }
89 }103 }
90104
105 Keys.onPressed: {
106 if (event.key == Qt.Key_Tab && event.modifiers == Qt.NoModifier) {
107 if (!folded) {
108 if (header.focus) {
109 options.forceActiveFocus()
110 options.currentIndex = 0
111 event.accepted = true
112 } else if (options.currentIndex + 1 < options.count) {
113 options.currentItem.focusHeader()
114 options.currentIndex++
115 event.accepted = true
116 }
117 }
118 } else if (event.key == Qt.Key_Backtab && event.modifiers == Qt.ShiftModifier) {
119 if (!folded && !header.focus) {
120 if (options.currentItem.header.activeFocus) {
121 if (options.currentIndex > 0) {
122 options.currentIndex--
123 options.currentItem.focusHeader()
124 } else {
125 header.forceActiveFocus()
126 }
127 } else {
128 options.currentItem.focusHeader()
129 }
130 event.accepted = true
131 }
132 }
133 }
134
91 ListView {135 ListView {
92 id: options136 id: options
93137
94138
=== modified file 'shell/dash/LensView.qml'
--- shell/dash/LensView.qml 2012-03-30 14:56:08 +0000
+++ shell/dash/LensView.qml 2012-04-16 13:48:20 +0000
@@ -28,6 +28,18 @@
28 property variant model28 property variant model
29 property string firstNonEmptyCategory29 property string firstNonEmptyCategory
3030
31 function focusFirstHeader() {
32 results.focusFirstHeader()
33 }
34
35 function focusLastHeader() {
36 results.focusLastHeader()
37 }
38
39 function isListEmpty() {
40 return results.isListEmpty()
41 }
42
31 function updateFirstCategory() {43 function updateFirstCategory() {
32 if (lensView.model.results.count == 0)44 if (lensView.model.results.count == 0)
33 return45 return
@@ -108,6 +120,18 @@
108 }120 }
109 }121 }
110122
123 Keys.onPressed: {
124 if (event.key == Qt.Key_Tab && event.modifiers == Qt.NoModifier) {
125 if (results.focusNextHeader()) {
126 event.accepted = true
127 }
128 } else if (event.key == Qt.Key_Backtab && event.modifiers == Qt.ShiftModifier) {
129 if (results.focusPreviousHeader()) {
130 event.accepted = true
131 }
132 }
133 }
134
111 ListViewWithScrollbar {135 ListViewWithScrollbar {
112 id: results136 id: results
113137
114138
=== modified file 'shell/dash/ListViewWithHeaders.qml'
--- shell/dash/ListViewWithHeaders.qml 2011-11-23 10:27:05 +0000
+++ shell/dash/ListViewWithHeaders.qml 2012-04-16 13:48:20 +0000
@@ -43,6 +43,46 @@
4343
44 property bool giveFocus: false44 property bool giveFocus: false
4545
46 function focusFirstHeader() {
47 focusPath.reset()
48 focusPath.currentItem.header.forceActiveFocus()
49 }
50
51 function focusLastHeader() {
52 focusPath.focusLastRow()
53 focusPath.currentItem.header.forceActiveFocus()
54 }
55
56 function focusNextHeader() {
57 var moved = focusPath.moveToNext()
58 if (moved) {
59 focusPath.currentItem.header.forceActiveFocus()
60 }
61 return moved
62 }
63
64 function focusPreviousHeader() {
65 if (!focusPath.currentItem.header.activeFocus) {
66 focusPath.currentItem.header.forceActiveFocus()
67 return true
68 } else {
69 var moved = focusPath.moveToPrevious()
70 if (moved) {
71 focusPath.currentItem.header.forceActiveFocus()
72 }
73 return moved
74 }
75 }
76
77 function isListEmpty() {
78 var empty = true
79 for (var i = 0; empty && i < categories.count; i++) {
80 var category = categories.itemAt(i);
81 empty = !category.body.visible && !category.header.visible
82 }
83 return empty
84 }
85
46 FocusPath {86 FocusPath {
47 id: focusPath87 id: focusPath
48 item: categoriesColumn88 item: categoriesColumn
@@ -75,7 +115,7 @@
75 var newContentY = -1;115 var newContentY = -1;
76116
77 if (scroll.contentY > itemTop) {117 if (scroll.contentY > itemTop) {
78 newContentY = itemTop118 newContentY = Math.max(0, itemTop)
79 } else if ((scroll.contentY + scroll.height) < itemBottom) {119 } else if ((scroll.contentY + scroll.height) < itemBottom) {
80 newContentY = itemBottom - scroll.height;120 newContentY = itemBottom - scroll.height;
81 }121 }
@@ -101,6 +141,9 @@
101 FocusPath.index: index141 FocusPath.index: index
102 FocusPath.skip: !headerLoader.item.visible && !bodyLoader.item.visible142 FocusPath.skip: !headerLoader.item.visible && !bodyLoader.item.visible
103143
144 property alias bodyLoader: bodyLoader
145 property alias body: bodyLoader.item
146 property alias header: headerLoader.item
104147
105 Column {148 Column {
106149
@@ -125,6 +168,13 @@
125168
126 onActiveFocusChanged: {169 onActiveFocusChanged: {
127 if (visible && item && item.activeFocus) {170 if (visible && item && item.activeFocus) {
171 var categoryOnTopIndex = focusPath.previousIndex()
172 if (categoryOnTopIndex != -1) {
173 categories.itemAt(categoryOnTopIndex).body.item.focusLastRow()
174 categories.itemAt(categoryOnTopIndex).bodyLoader.focus = true
175 }
176
177 focusPath.currentItem.body.item.focusFirstElement()
128 scroll.moveToPosition(item)178 scroll.moveToPosition(item)
129 }179 }
130 }180 }
131181
=== modified file 'shell/dash/ListViewWithScrollbar.qml'
--- shell/dash/ListViewWithScrollbar.qml 2011-12-07 14:57:53 +0000
+++ shell/dash/ListViewWithScrollbar.qml 2012-04-16 13:48:20 +0000
@@ -25,6 +25,26 @@
25 property alias bodyDelegate: list.bodyDelegate25 property alias bodyDelegate: list.bodyDelegate
26 property alias headerDelegate: list.headerDelegate26 property alias headerDelegate: list.headerDelegate
2727
28 function focusFirstHeader() {
29 list.focusFirstHeader()
30 }
31
32 function focusLastHeader() {
33 list.focusLastHeader()
34 }
35
36 function focusNextHeader() {
37 return list.focusNextHeader()
38 }
39
40 function focusPreviousHeader() {
41 return list.focusPreviousHeader()
42 }
43
44 function isListEmpty() {
45 return list.isListEmpty()
46 }
47
28 ListViewWithHeaders {48 ListViewWithHeaders {
29 id: list49 id: list
3050
3151
=== modified file 'shell/dash/RendererGrid.qml'
--- shell/dash/RendererGrid.qml 2012-02-10 20:37:18 +0000
+++ shell/dash/RendererGrid.qml 2012-04-16 13:48:20 +0000
@@ -43,6 +43,14 @@
4343
44 property bool centered: true44 property bool centered: true
4545
46 function focusFirstElement() {
47 focusPath.reset()
48 }
49
50 function focusLastRow() {
51 focusPath.focusLastRow()
52 }
53
46 FocusPath {54 FocusPath {
47 id: focusPath55 id: focusPath
4856
4957
=== modified file 'tests/manual-tests/dash.txt'
--- tests/manual-tests/dash.txt 2012-04-11 13:46:44 +0000
+++ tests/manual-tests/dash.txt 2012-04-16 13:48:20 +0000
@@ -127,6 +127,107 @@
127-----127-----
128128
129 # Test case objectives:129 # Test case objectives:
130 # * Check that Tab moves between headers on the dash
131 # Pre-conditions
132 # * Have a files and folders lens with 3 categories
133 # Test steps
134 # * Open dash
135 # * Switch to the files and folders lens
136 # * Verify focus is on the search entry
137 # * Press Tab
138 # * Verify focus is on the first category header
139 # * Press Tab
140 # * Verify focus is on the second category header
141 # * Press Tab
142 # * Verify focus is on the third category header
143 # * Press Tab
144
145-----
146
147 # Test case objectives:
148 # * Check that Tab moves between headers and Filters on the dash
149 # Pre-conditions
150 # * Have a files and folders lens with 3 categories
151 # * Have the focus on the third header of files and folders lens
152 # * Have the filters collapsed
153 # Test steps
154 # * Verify focus is on the "Filter results" text
155 # * Press Tab
156 # * Verify focus is on the first category header
157
158-----
159
160 # Test case objectives:
161 # * Check that Tab moves between Filters "All" buttons
162 # Pre-conditions
163 # * Be on the files and folders lens
164 # * Have the focus on the "Filter results" text
165 # * Have the filters collapsed
166 # Test steps
167 # * Verify focus is on the "Filter results" text
168 # * Press Enter to expand the filters
169 # * Press Tab
170 # * Verify focus is on the "Last Modified" filter "All" button
171 # * Press Tab
172 # * Verify focus is on the "Type" filter "All" button
173 # * Press Tab
174 # * Verify focus is on the "Size" filter "All" button
175 # * Press Tab
176 # * Verify focus is on the first category header
177
178-----
179
180 # Test case objectives:
181 # * Check that Tab goes to the next category header when not on the header
182 # Pre-conditions
183 # * Be on the files and folders lens
184 # * Have the focus on the first category header
185 # Test steps
186 # * Press Down Arrow
187 # * Verify focus is on the first element of the first category
188 # * Press Tab
189 # * Verify focus is on the second category header
190
191-----
192
193 # Test case objectives:
194 # * Check that Shift+Tab goes to the current category header
195 # Pre-conditions
196 # * Be on the files and folders lens
197 # * Have the focus on the first category header
198 # Test steps
199 # * Press Down Arrow
200 # * Verify focus is on the first element of the first category
201 # * Press Shift+Tab
202 # * Verify focus is on the first category header
203
204-----
205
206 # Test case objectives:
207 # * Check that Shift+Tab cycles through category headers and filters
208 # Pre-conditions
209 # * Be on the files and folders lens
210 # * Have the filters uncollapsed
211 # * Have the focus on the third category header
212 # Test steps
213 # * Press Shift+Tab
214 # * Verify focus is on the second category header
215 # * Press Shift+Tab
216 # * Verify focus is on the first category header
217 # * Press Shift+Tab
218 # * Verify focus is on the "Size" filter "All" button
219 # * Press Shift+Tab
220 # * Verify focus is on the "Type" filter "All" button
221 # * Press Shift+Tab
222 # * Verify focus is on the "Last Modified" filter "All" button
223 # * Press Shift+Tab
224 # * Verify focus is on the "Filter results" text
225 # * Press Shift+Tab
226 # * Verify focus is on the third category header
227
228-----
229
230# Test case objectives:
130 # * Verify the 4 finger tap gesture toggles the dash231 # * Verify the 4 finger tap gesture toggles the dash
131 # Pre-conditions232 # Pre-conditions
132 # * None233 # * None

Subscribers

People subscribed via source and target branches