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

[Paweł Stołowski]

[hud] Prevent search entry from stealing focus on mouse click, rendering up/down keys unusable.

Up/Down keys update currentIndex of resultsList. Changed color & width property code in ResultItem to report color/width based on whether it's current item, rather than relying on "selected" state of AbstractButton.

[hud] Prevent search entry from stealing focus on mouse click, rendering up/down keys unusable.
   Up/Down keys update currentIndex of resultsList. Changed color & width property code in ResultItem
   to report color/width based on whether it's current item, rather than relying on "selected" state
   of AbstractButton.

[Albert Astals Cid (aacid)]

Wondering if
verify_equal('false', TIMEOUT, "Result item ##{index+1} should not have focus") {
we can have something that tests the item is the current one (using that current property)?

Also you don't need focus: false since it's the default value, so you can just remove it I think (unless you want to explicitely have it there)

review: Needs Fixing

[lp:~stolowski/unity-2d/hud-focus-fix]

1042. By Paweł Stołowski on 2012-04-04

Check for 'pressed' state first to highlight current item correctly on click.

1043. By Paweł Stołowski on 2012-04-04

Test 'current' property of ResultItem in tests.

[Albert Astals Cid (aacid)]

Looks great

review: Approve

=== modified file 'shell/hud/Hud.qml'
--- shell/hud/Hud.qml	2012-04-04 07:46:44 +0000
+++ shell/hud/Hud.qml	2012-04-04 08:50:24 +0000
@@ -58,7 +58,6 @@
         if (active) {
             shellManager.hudShell.forceActivateWindow()
             appIcon = getActiveWindowIcon()
-            resultList.focus = true
         } else {
             hudModel.endSearch()
             resultList.currentIndex = -1
@@ -78,6 +77,12 @@
 
     Keys.onPressed: {
         if (event.key == Qt.Key_Escape) toggleHud()
+        else if (event.key == Qt.Key_Down) {
+            resultList.incrementCurrentIndex()
+        }
+        else if (event.key == Qt.Key_Up) {
+            resultList.decrementCurrentIndex()
+        }
     }
 
     function toggleHud() {
@@ -193,6 +198,7 @@
 
             SearchEntry {
                 id: searchEntry
+                focus: true
 
                 anchors.top: parent.top
                 anchors.left: parent.left
@@ -215,7 +221,7 @@
             ListView {
                 id: resultList
 
-                focus: true
+                focus: false
 
                 Accessible.name: "result list"
 
@@ -234,12 +240,12 @@
 
                     icon: iconName /* expose this property for tile */
 
+                    current: ListView.isCurrentItem
+
                     onClicked: executeResult(resultId)
                     onMouseOverChanged: {
                         if (mouseOver) {
                             resultList.currentIndex = model.index;
-                            // workaround for loosing highlight for mouse if search entry steals focus - see https://bugs.launchpad.net/unity-2d/+bug/966180
-                            forceActiveFocus();
                         }
                     }
                 }

=== modified file 'shell/hud/ResultItem.qml'
--- shell/hud/ResultItem.qml	2012-04-03 16:30:19 +0000
+++ shell/hud/ResultItem.qml	2012-04-04 08:50:24 +0000
@@ -21,6 +21,7 @@
 
 AbstractButton {
     id: delegate
+    property bool current: false
     property string icon: ""
 
     Accessible.name: plainText
@@ -47,11 +48,10 @@
         anchors.bottom: parent.bottom
 
         border.color: "white"
-        border.width: (parent.state == "selected"
-                       || parent.state == "pressed") ? 2 : 0
+        border.width: (current || parent.state == "pressed") ? 2 : 0
         color: {
-            if (parent.state == "selected") return "#21ffffff"
-            else if ( parent.state == "pressed" ) return "white"
+            if ( parent.state == "pressed" ) return "white"
+            else if (current) return "#21ffffff"
             else return "transparent"
         }
         radius: 7

=== modified file 'tests/hud/hud-search-tests.rb'
--- tests/hud/hud-search-tests.rb	2012-04-03 16:11:22 +0000
+++ tests/hud/hud-search-tests.rb	2012-04-04 08:50:24 +0000
@@ -43,8 +43,8 @@
 #
 # Verify that given ResultItem is highlighted and active.
 def item_is_highlighted(results, index)
-    verify_equal('true', TIMEOUT, "Result item ##{index+1} should have focus") {
-        results[index]['activeFocus']
+    verify_equal('true', TIMEOUT, "Result item ##{index+1} should be 'current'") {
+        results[index]['current']
     }
 
     verify_equal(HUD_HIGHLIGHT_COLOR, TIMEOUT, "Result item ##{index+1} should be highlighted") {
@@ -59,12 +59,8 @@
         results.count {|x| x.QDeclarativeRectangle(:name=>'container')['color'] == HUD_HIGHLIGHT_COLOR}
     }
 
-    verify_equal(1, 0, 'Exactly one item has focus') {
-        results.count {|x| x['activeFocus'] == 'true'}
-    }
-
-    verify_equal(1, 0, "Exactly one item has 'selected' state ") {
-        results.count {|x| x['state'] == 'selected'}
+    verify_equal(1, 0, "Exactly one item is 'current'") {
+        results.count {|x| x['current'] == 'true'}
     }
 end