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

Proposed by Zsombor Egri
Status: Merged
Approved by: Cris Dywan
Approved revision: 1620
Merged at revision: 1622
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/listItemHandleUnacceptedMouseEvent
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 121 lines (+35/-18)
4 files modified
src/Ubuntu/Components/plugin/uclistitem.cpp (+6/-16)
src/Ubuntu/Components/plugin/uclistitem_p.h (+1/-1)
tests/unit_x11/tst_components/ListItemTestCase.qml (+1/-0)
tests/unit_x11/tst_components/tst_listitem_extras.qml (+27/-1)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/listItemHandleUnacceptedMouseEvent
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Cris Dywan Approve
Review via email: mp+269607@code.launchpad.net

Commit message

Fix ListItem swipe handling when gesture is initiated over an overlay MouseArea which does not accept pressed event.

To post a comment you must log in.
Revision history for this message
Cris Dywan (kalikiana) wrote :

That makes a whole lot of sense. A MouseArea really shouldn't have a say in accepting the swipe handling.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Ubuntu/Components/plugin/uclistitem.cpp'
2--- src/Ubuntu/Components/plugin/uclistitem.cpp 2015-07-30 13:27:32 +0000
3+++ src/Ubuntu/Components/plugin/uclistitem.cpp 2015-08-31 07:54:47 +0000
4@@ -372,23 +372,13 @@
5 (parentItem ? QQuickItemPrivate::get(parentItem)->childItems.indexOf(q) : -1);
6 }
7
8-// returns true if the highlight is possible
9-bool UCListItemPrivate::canHighlight(QMouseEvent *event)
10+// returns true if the highlight is possible; the highlight is possible if the
11+// list item has at least one action, leading/trailing actions set, onClicked
12+// or onPressAndHold signal handlers set
13+bool UCListItemPrivate::canHighlight()
14 {
15- // if automatic, the highlight should not happen if we clicked on an active component;
16- // localPos is a position relative to ListItem which will give us a child from
17- // from the original coordinates; therefore we must map the position to the contentItem
18 Q_Q(UCListItem);
19- QPointF myPos = q->mapToItem(contentItem, event->localPos());
20- QQuickItem *child = contentItem->childAt(myPos.x(), myPos.y());
21- bool activeComponent = child && (child->acceptedMouseButtons() & event->button()) &&
22- child->isEnabled() && !qobject_cast<QQuickText*>(child);
23- // do highlight if not pressed above the active component, and the component has either
24- // action, leading or trailing actions list or at least an active child component declared
25- QQuickMouseArea *ma = q->findChild<QQuickMouseArea*>();
26- bool activeMouseArea = ma && ma->isEnabled();
27- return !activeComponent && (isClickedConnected() || isPressAndHoldConnected() ||
28- mainAction || leadingActions || trailingActions || activeMouseArea);
29+ return (isClickedConnected() || isPressAndHoldConnected() || mainAction || leadingActions || trailingActions);
30 }
31
32 // set highlighted flag and update contentItem
33@@ -1081,7 +1071,7 @@
34 // while moving, we cannot select any items
35 return;
36 }
37- if (d->canHighlight(event) && !d->highlighted && event->button() == Qt::LeftButton) {
38+ if (d->canHighlight() && !d->highlighted && event->button() == Qt::LeftButton) {
39 d->grabLeftButtonEvents(event);
40 }
41 // accept the event so we get the rest of the events as well
42
43=== modified file 'src/Ubuntu/Components/plugin/uclistitem_p.h'
44--- src/Ubuntu/Components/plugin/uclistitem_p.h 2015-07-30 13:27:32 +0000
45+++ src/Ubuntu/Components/plugin/uclistitem_p.h 2015-08-31 07:54:47 +0000
46@@ -60,7 +60,7 @@
47 void _q_syncSelectMode();
48 void _q_syncDragMode();
49 int index();
50- bool canHighlight(QMouseEvent *event);
51+ bool canHighlight();
52 void setHighlighted(bool pressed);
53 void listenToRebind(bool listen);
54 void lockContentItem(bool lock);
55
56=== modified file 'tests/unit_x11/tst_components/ListItemTestCase.qml'
57--- tests/unit_x11/tst_components/ListItemTestCase.qml 2015-07-31 06:55:59 +0000
58+++ tests/unit_x11/tst_components/ListItemTestCase.qml 2015-08-31 07:54:47 +0000
59@@ -34,6 +34,7 @@
60 if (item.hasOwnProperty("leadingActions")) {
61 signalSpy.target = item;
62 signalSpy.signalName = signalName;
63+ signalSpy.clear();
64 }
65 }
66 // wait on the previosuly set up spy
67
68=== modified file 'tests/unit_x11/tst_components/tst_listitem_extras.qml'
69--- tests/unit_x11/tst_components/tst_listitem_extras.qml 2015-08-04 20:09:10 +0000
70+++ tests/unit_x11/tst_components/tst_listitem_extras.qml 2015-08-31 07:54:47 +0000
71@@ -68,10 +68,22 @@
72 anchors.centerIn: parent
73 }
74 }
75+
76+ ListItem {
77+ id: overlaidMouseArea
78+ leadingActions: leading
79+ trailingActions: trailing
80+ property bool acceptEvent
81+ MouseArea {
82+ id: overlayArea
83+ anchors.fill: parent
84+ onPressed: mouse.accepted = overlaidMouseArea.acceptEvent
85+ }
86+ }
87 }
88
89 ListItemTestCase {
90-
91+ name: "ListItemExtras"
92 SignalSpy {
93 id: clickSpy
94 signalName: "clicked"
95@@ -79,6 +91,7 @@
96
97 function cleanup() {
98 rebound(testWithActiveItem);
99+ rebound(overlaidMouseArea);
100 clickSpy.target = null;
101 clickSpy.clear();
102 }
103@@ -88,5 +101,18 @@
104 swipe(activeItem, centerOf(activeItem).x, centerOf(activeItem).y, units.gu(10));
105 compare(clickSpy.count, 0, "activeItem was clicked");
106 }
107+
108+ function test_swipe_over_mousearea_not_accepting_press_data() {
109+ return [
110+ {tag: "Accept events", accept: true},
111+ {tag: "Do not accept events", accept: false},
112+ ];
113+ }
114+ function test_swipe_over_mousearea_not_accepting_press(data) {
115+ overlaidMouseArea.acceptEvent = data.accept;
116+ setupSpy(overlaidMouseArea, "contentMovementEnded");
117+ swipeNoWait(overlayArea, centerOf(overlayArea).x, centerOf(overlayArea).y, units.gu(10));
118+ spyWait();
119+ }
120 }
121 }

Subscribers

People subscribed via source and target branches