Merge lp:~zsombi/ubuntu-ui-toolkit/listItemFocusRegression into lp:ubuntu-ui-toolkit/staging

Proposed by Zsombor Egri on 2016-10-06
Status: Merged
Approved by: Christian Dywan on 2016-10-12
Approved revision: 2139
Merged at revision: 2141
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/listItemFocusRegression
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 56 lines (+19/-2)
2 files modified
src/UbuntuToolkit/privates/listviewextensions.cpp (+2/-0)
tests/unit/visual/tst_listitem_focus_bug.13.qml (+17/-2)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/listItemFocusRegression
Reviewer Review Type Date Requested Status
ubuntu-sdk-build-bot continuous-integration Approve on 2016-10-12
Christian Dywan (community) 2016-10-06 Approve on 2016-10-12
Review via email: mp+307822@code.launchpad.net

Commit message

Do not show focus frame on ListItem when clicked.

Description of the change

Do not show focus frame on ListItem when clicked.

To post a comment you must log in.
Christian Dywan (kalikiana) wrote :

> 8 + , isEventFilter(false)
> 9 + , keyNavigation(false)

I'm dreaming of GCC properly failing builds with warnings, which are triggered consistently - or perhaps we need to switch to building with clang eventually.

Nice catch in any case.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/UbuntuToolkit/privates/listviewextensions.cpp'
2--- src/UbuntuToolkit/privates/listviewextensions.cpp 2016-09-16 05:01:13 +0000
3+++ src/UbuntuToolkit/privates/listviewextensions.cpp 2016-10-11 12:18:36 +0000
4@@ -31,6 +31,8 @@
5 : QObject(parent)
6 , listView(listView)
7 , _currentItem(Q_NULLPTR)
8+ , isEventFilter(false)
9+ , keyNavigation(false)
10 {
11 connect(listView, SIGNAL(currentItemChanged()), this, SLOT(onCurrentItemChanged()), Qt::DirectConnection);
12 onCurrentItemChanged();
13
14=== modified file 'tests/unit/visual/tst_listitem_focus_bug.13.qml'
15--- tests/unit/visual/tst_listitem_focus_bug.13.qml 2016-09-08 10:21:35 +0000
16+++ tests/unit/visual/tst_listitem_focus_bug.13.qml 2016-10-11 12:18:36 +0000
17@@ -29,22 +29,37 @@
18 id: testView
19 anchors.fill: parent
20 model: 50
21- currentIndex: 10
22+ currentIndex: -1
23 delegate: ListItem { width: testView.width; height: units.gu(7)
24 objectName: "listItem" + index
25- enabled: index > 20 && index < 25
26+ enabled: index == 4 || (index > 20 && index < 25)
27 Label {
28 anchors.centerIn: parent
29 text: (parent.enabled ? "enabled" : "disabled") + index
30 }
31+ onClicked: testView.currentIndex = index
32 }
33 }
34
35+ SignalSpy {
36+ id: currentItemSpy
37+ target: testView
38+ signalName: "currentItemChanged"
39+ }
40+
41 UbuntuTestCase {
42 when: windowShown
43
44 function cleanup() {
45 main.forceActiveFocus();
46+ currentItemSpy.clear();
47+ }
48+
49+ function test_0_first_time_focus_on_listview_bug1628855() {
50+ var listItem = findChild(testView, "listItem4");
51+ mouseClick(listItem, centerOf(listItem).x, centerOf(listItem).y);
52+ currentItemSpy.wait();
53+ compare(listItem.keyNavigationFocus, false);
54 }
55
56 function test_focus_bug1611327_data() {

Subscribers

People subscribed via source and target branches