Merge lp:~mterry/ubuntu-ui-toolkit/monitor-only into lp:ubuntu-ui-toolkit/staging

Proposed by Michael Terry
Status: Merged
Approved by: Zsombor Egri
Approved revision: 1281
Merged at revision: 1886
Proposed branch: lp:~mterry/ubuntu-ui-toolkit/monitor-only
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 314 lines (+179/-6)
6 files modified
components.api (+2/-0)
src/Ubuntu/UbuntuGestures/ucswipearea.cpp (+52/-5)
src/Ubuntu/UbuntuGestures/ucswipearea_p.h (+5/-0)
src/Ubuntu/UbuntuGestures/ucswipearea_p_p.h (+1/-0)
tests/unit_x11/tst_swipearea/DownwardsLauncher.qml (+14/-0)
tests/unit_x11/tst_swipearea/tst_swipearea.cpp (+105/-1)
To merge this branch: bzr merge lp:~mterry/ubuntu-ui-toolkit/monitor-only
Reviewer Review Type Date Requested Status
ubuntu-sdk-build-bot continuous-integration Approve
Zsombor Egri Approve
Timo Jyrinki trigger ci for non-sdk-team member Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+282034@code.launchpad.net

This proposal supersedes a proposal from 2015-12-17.

Commit message

Add grabGesture field to SwipeArea.
This field controls whether the SwipeArea takes ownership of the gestures it observes.

Description of the change

From linked bug:

== Ask ==

I want to add SwipeArea.grabGesture which lets input events "fall through" (i.e. it doesn't own them) but continues to monitor them.

I have a specific need for it, but it might be a generally useful thing?

== Backstory ==

Recently, the UITK got SwipeArea, which is great. In unity8, I'm working on a redesign of the edge introduction tutorial on first boot of your device.

There's a part of it where we put a shell-wide overlay on top of the screen that says "Swipe from the bottom to do ..." and this should fade out as the user swipes up (but should do the right thing if the drag isn't completed or is held in place).

One way to do this would be to communicate with the app. But that's a lot of infrastructure for one little feature. And this feature might be short-lived, because eventually we're going to try to move those "coach mark" help screens in the UITK as well for apps to use themselves. But for now, the shell is showing the bottom edge coach mark for a select few apps.

So another easy way to do it is to be able to monitor the user's drag, but not interfere with it. So the shell can see what the drag is doing, but the app still does the drag itself.

== Next Steps ==

I'd like this branch reviewed for sanity, to see if it's something you folks are OK with. It's missing tests, but I'm waiting to add those until you all let me know it wouldn't be wasted effort. :)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal

I think grabGesture as property name would be better than monitorOnly.

Secondly, please submit your MR agains UITK staging lp:ubuntu-ui-toolkit/staging.

review: Needs Fixing
Revision history for this message
Michael Terry (mterry) wrote :

OK, renamed monitorOnly to grabGesture (and inverted meaning obviously). And I've resubmitted against the staging branch.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1278. By Michael Terry

Fix components.api

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Timo Jyrinki (timo-jyrinki) :
review: Approve (trigger ci for non-sdk-team member)
Revision history for this message
Michael Terry (mterry) wrote :

Poke

Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Zsombor Egri (zsombi) wrote :

I was about to comment about the default value of the grabGesture, but then I realised the SwipeArea was already grabbing the gesture, so the default value is fine.

Few tiny things inline, and tests please, then we are good. Ping me when you are ready with.

review: Needs Fixing
1279. By Michael Terry

Merge from staging

1280. By Michael Terry

Fix style

1281. By Michael Terry

Add a couple grabGesture tests

Revision history for this message
Michael Terry (mterry) wrote :

OK, updated to address your inline comments and add a couple tests. Please re-review, thanks!

Revision history for this message
Zsombor Egri (zsombi) wrote :

Much better now , thanks!

review: Approve
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) 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 'components.api'
2--- components.api 2016-03-02 10:06:30 +0000
3+++ components.api 2016-03-07 15:56:30 +0000
4@@ -1177,6 +1177,7 @@
5 property Direction direction
6 readonly property double distance
7 readonly property bool dragging
8+ property bool grabGesture
9 property bool immediateRecognition
10 signal directionChanged(Direction direction)
11 signal draggingChanged(bool dragging)
12@@ -1184,6 +1185,7 @@
13 signal distanceChanged(double distance)
14 signal touchPositionChanged(QPointF position)
15 signal immediateRecognitionChanged(bool immediateRecognition)
16+ signal grabGestureChanged(bool grabGesture)
17 readonly property bool pressed
18 readonly property QPointF touchPosition
19 Ubuntu.Components.SwipeArea.Direction: Enum
20
21=== modified file 'src/Ubuntu/UbuntuGestures/ucswipearea.cpp'
22--- src/Ubuntu/UbuntuGestures/ucswipearea.cpp 2016-01-11 09:27:19 +0000
23+++ src/Ubuntu/UbuntuGestures/ucswipearea.cpp 2016-03-07 15:56:30 +0000
24@@ -352,6 +352,38 @@
25 Q_EMIT immediateRecognitionChanged(enabled);
26 }
27 }
28+
29+/*!
30+ * \qmlproperty bool SwipeArea::grabGesture
31+ * If true, any gestures will be grabbed and owned by the SwipeArea as usual.
32+ * If false, gestures will still be reported, but events may be grabbed by
33+ * another Qml object.
34+ *
35+ * Defaults to true. In most cases this should not be unset.
36+ */
37+bool UCSwipeArea::grabGesture() const
38+{
39+ Q_D(const UCSwipeArea);
40+ return d->grabGesture;
41+}
42+
43+void UCSwipeArea::setGrabGesture(bool enabled)
44+{
45+ Q_D(UCSwipeArea);
46+ if (d->grabGesture == enabled) {
47+ return;
48+ }
49+
50+ d->grabGesture = enabled;
51+
52+ if (!d->grabGesture && d->status == UCSwipeAreaPrivate::Undecided) {
53+ TouchRegistry::instance()->removeCandidateOwnerForTouch(d->touchId, this);
54+ // We still wanna know when it ends for keeping the composition time window up-to-date
55+ TouchRegistry::instance()->addTouchWatcher(d->touchId, this);
56+ }
57+
58+ Q_EMIT grabGestureChanged(enabled);
59+}
60
61 bool UCSwipeArea::event(QEvent *event)
62 {
63@@ -401,7 +433,10 @@
64 unownedTouchEvent_undecided(unownedTouchEvent);
65 break;
66 default: // Recognized:
67- // do nothing
68+ if (!grabGesture) {
69+ // Treat unowned event as if we owned it, but we are really just watching it
70+ touchEvent_recognized(event);
71+ }
72 break;
73 }
74
75@@ -453,7 +488,9 @@
76 }
77
78 if (movedFarEnoughAlongGestureAxis()) {
79- TouchRegistry::instance()->requestTouchOwnership(touchId, q);
80+ if (grabGesture) {
81+ TouchRegistry::instance()->requestTouchOwnership(touchId, q);
82+ }
83 setStatus(Recognized);
84 updatePosition(touchScenePosition);
85 } else if (isPastMaxDistance()) {
86@@ -552,12 +589,21 @@
87 if (recognitionIsDisabled()) {
88 // Behave like a dumb TouchArea
89 SA_TRACE("Gesture recognition is disabled. Requesting touch ownership immediately.");
90- TouchRegistry::instance()->requestTouchOwnership(touchId, q);
91 setStatus(Recognized);
92- event->accept();
93+ if (grabGesture) {
94+ TouchRegistry::instance()->requestTouchOwnership(touchId, q);
95+ event->accept();
96+ } else {
97+ watchPressedTouchPoints(touchPoints);
98+ event->ignore();
99+ }
100 } else {
101 // just monitor the touch points for now.
102- TouchRegistry::instance()->addCandidateOwnerForTouch(touchId, q);
103+ if (grabGesture) {
104+ TouchRegistry::instance()->addCandidateOwnerForTouch(touchId, q);
105+ } else {
106+ watchPressedTouchPoints(touchPoints);
107+ }
108
109 setStatus(Undecided);
110 // Let the item below have it. We will monitor it and grab it later if a gesture
111@@ -984,6 +1030,7 @@
112 , status(WaitingForTouch)
113 , direction(UCSwipeArea::Rightwards)
114 , immediateRecognition(false)
115+ , grabGesture(true)
116 {
117 }
118
119
120=== modified file 'src/Ubuntu/UbuntuGestures/ucswipearea_p.h'
121--- src/Ubuntu/UbuntuGestures/ucswipearea_p.h 2015-12-17 15:23:26 +0000
122+++ src/Ubuntu/UbuntuGestures/ucswipearea_p.h 2016-03-07 15:56:30 +0000
123@@ -46,6 +46,7 @@
124 READ immediateRecognition
125 WRITE setImmediateRecognition
126 NOTIFY immediateRecognitionChanged)
127+ Q_PROPERTY(bool grabGesture READ grabGesture WRITE setGrabGesture NOTIFY grabGestureChanged FINAL)
128
129 Q_ENUMS(Direction)
130 public:
131@@ -74,6 +75,9 @@
132 bool immediateRecognition() const;
133 void setImmediateRecognition(bool enabled);
134
135+ bool grabGesture() const;
136+ void setGrabGesture(bool enabled);
137+
138 Q_SIGNALS:
139 void directionChanged(Direction direction);
140 void draggingChanged(bool dragging);
141@@ -81,6 +85,7 @@
142 void distanceChanged(qreal distance);
143 void touchPositionChanged(const QPointF &position);
144 void immediateRecognitionChanged(bool immediateRecognition);
145+ void grabGestureChanged(bool grabGesture);
146
147 protected:
148 bool event(QEvent *e) override;
149
150=== modified file 'src/Ubuntu/UbuntuGestures/ucswipearea_p_p.h'
151--- src/Ubuntu/UbuntuGestures/ucswipearea_p_p.h 2015-12-17 15:23:26 +0000
152+++ src/Ubuntu/UbuntuGestures/ucswipearea_p_p.h 2016-03-07 15:56:30 +0000
153@@ -162,6 +162,7 @@
154 UCSwipeArea::Direction direction;
155
156 bool immediateRecognition;
157+ bool grabGesture;
158 };
159
160 class UBUNTUGESTURES_EXPORT UCSwipeAreaStatusListener
161
162=== modified file 'tests/unit_x11/tst_swipearea/DownwardsLauncher.qml'
163--- tests/unit_x11/tst_swipearea/DownwardsLauncher.qml 2015-11-17 12:20:03 +0000
164+++ tests/unit_x11/tst_swipearea/DownwardsLauncher.qml 2016-03-07 15:56:30 +0000
165@@ -63,6 +63,20 @@
166 }
167 }
168
169+ SwipeArea {
170+ id: swipeAreaNoGrab
171+ objectName: "vpDragAreaNoGrab"
172+
173+ grabGesture: false
174+ height: swipeArea.height
175+ direction: swipeArea.direction
176+ anchors {
177+ left: swipeArea.left
178+ right: swipeArea.right
179+ top: swipeArea.top
180+ }
181+ }
182+
183 Label {
184 text: "Downwards"
185 anchors.top: parent.top
186
187=== modified file 'tests/unit_x11/tst_swipearea/tst_swipearea.cpp'
188--- tests/unit_x11/tst_swipearea/tst_swipearea.cpp 2015-12-17 15:23:26 +0000
189+++ tests/unit_x11/tst_swipearea/tst_swipearea.cpp 2016-03-07 15:56:30 +0000
190@@ -1,5 +1,5 @@
191 /*
192- * Copyright 2015 Canonical Ltd.
193+ * Copyright 2015-2016 Canonical Ltd.
194 *
195 * This program is free software; you can redistribute it and/or modify
196 * it under the terms of the GNU Lesser General Public License as published by
197@@ -124,6 +124,8 @@
198 void makoRightEdgeDrag_verticalDownwards();
199 void makoLeftEdgeDrag_slowStart();
200 void makoLeftEdgeDrag_movesSlightlyBackwardsOnStart();
201+ void grabGesture();
202+ void grabGestureWithImmediateRecognition();
203
204 private:
205 // QTest::touchEvent takes QPoint instead of QPointF and I don't want to
206@@ -1338,6 +1340,108 @@
207 delete statusSpy;
208 }
209
210+void tst_UCSwipeArea::grabGesture()
211+{
212+ UCSwipeArea *edgeDragArea =
213+ m_view->rootObject()->findChild<UCSwipeArea*>("vpDragArea");
214+ Q_ASSERT(edgeDragArea != 0);
215+ UCSwipeAreaPrivate *d = UCSwipeAreaPrivate::get(edgeDragArea);
216+ d->setRecognitionTimer(m_fakeTimerFactory->createTimer(edgeDragArea));
217+ d->setTimeSource(m_fakeTimerFactory->timeSource());
218+
219+ UCSwipeArea *edgeDragAreaNoGrab =
220+ m_view->rootObject()->findChild<UCSwipeArea*>("vpDragAreaNoGrab");
221+ Q_ASSERT(edgeDragAreaNoGrab != 0);
222+ QCOMPARE(edgeDragAreaNoGrab->grabGesture(), false);
223+ UCSwipeAreaPrivate *dNoGrab = UCSwipeAreaPrivate::get(edgeDragAreaNoGrab);
224+ dNoGrab->setRecognitionTimer(m_fakeTimerFactory->createTimer(edgeDragAreaNoGrab));
225+ dNoGrab->setTimeSource(m_fakeTimerFactory->timeSource());
226+
227+ QPointF touchPoint = calculateInitialtouchPosition(edgeDragArea);
228+
229+ qreal desiredDragDistance = d->distanceThreshold * 2.0f;
230+ QPointF dragDirectionVector(0., 1.); // vertical positive
231+
232+ qreal movementStepDistance = d->distanceThreshold * 0.1f;
233+ QPointF touchMovement = dragDirectionVector * movementStepDistance;
234+ int totalMovementSteps = qCeil(desiredDragDistance / movementStepDistance);
235+ int movementTimeStepMs = (d->compositionTime * 1.5f) / totalMovementSteps;
236+
237+ qint64 timestamp = 0;
238+
239+ sendTouchPress(timestamp, 0, touchPoint);
240+
241+ for (int i = 0; i < totalMovementSteps; ++i) {
242+ touchPoint += touchMovement;
243+ timestamp += movementTimeStepMs;
244+ sendTouchUpdate(timestamp, 0, touchPoint);
245+ }
246+
247+ QCOMPARE((int)d->status, (int)UCSwipeAreaPrivate::Recognized);
248+ QCOMPARE(edgeDragArea->dragging(), true);
249+
250+ QCOMPARE((int)dNoGrab->status, (int)UCSwipeAreaPrivate::Recognized);
251+ QCOMPARE(edgeDragAreaNoGrab->dragging(), true);
252+
253+ // Use a large distance margin because position smoothing makes it hard
254+ // to assume high accuracy. The test below using immediateRecognition uses
255+ // a smaller margin.
256+ // NB: qFuzzyCompare(), used internally by QCOMPARE(), is broken.
257+ QVERIFY(qAbs(edgeDragArea->distance() - edgeDragAreaNoGrab->distance()) < 2);
258+
259+ timestamp += movementTimeStepMs;
260+ sendTouchRelease(timestamp, 0, touchPoint);
261+}
262+
263+void tst_UCSwipeArea::grabGestureWithImmediateRecognition()
264+{
265+ UCSwipeArea *edgeDragArea =
266+ m_view->rootObject()->findChild<UCSwipeArea*>("vpDragArea");
267+ Q_ASSERT(edgeDragArea != 0);
268+ UCSwipeAreaPrivate *d = UCSwipeAreaPrivate::get(edgeDragArea);
269+
270+ UCSwipeArea *edgeDragAreaNoGrab =
271+ m_view->rootObject()->findChild<UCSwipeArea*>("vpDragAreaNoGrab");
272+ Q_ASSERT(edgeDragAreaNoGrab != 0);
273+ QCOMPARE(edgeDragAreaNoGrab->grabGesture(), false);
274+ UCSwipeAreaPrivate *dNoGrab = UCSwipeAreaPrivate::get(edgeDragAreaNoGrab);
275+
276+ edgeDragArea->setImmediateRecognition(true);
277+ edgeDragAreaNoGrab->setImmediateRecognition(true);
278+
279+ QPointF touchPoint = calculateInitialtouchPosition(edgeDragArea);
280+
281+ qreal desiredDragDistance = d->distanceThreshold * 2.0f;
282+ QPointF dragDirectionVector(0., 1.); // vertical positive
283+
284+ qreal movementStepDistance = d->distanceThreshold * 0.1f;
285+ QPointF touchMovement = dragDirectionVector * movementStepDistance;
286+ int totalMovementSteps = qCeil(desiredDragDistance / movementStepDistance);
287+ int movementTimeStepMs = (d->compositionTime * 1.5f) / totalMovementSteps;
288+
289+ qint64 timestamp = 0;
290+
291+ sendTouchPress(timestamp, 0, touchPoint);
292+
293+ for (int i = 0; i < totalMovementSteps; ++i) {
294+ touchPoint += touchMovement;
295+ timestamp += movementTimeStepMs;
296+ sendTouchUpdate(timestamp, 0, touchPoint);
297+ }
298+
299+ QCOMPARE((int)d->status, (int)UCSwipeAreaPrivate::Recognized);
300+ QCOMPARE(edgeDragArea->dragging(), true);
301+
302+ QCOMPARE((int)dNoGrab->status, (int)UCSwipeAreaPrivate::Recognized);
303+ QCOMPARE(edgeDragAreaNoGrab->dragging(), true);
304+
305+ // NB: qFuzzyCompare(), used internally by QCOMPARE(), is broken.
306+ QVERIFY(qAbs(edgeDragArea->distance() - edgeDragAreaNoGrab->distance()) < 0.001);
307+
308+ timestamp += movementTimeStepMs;
309+ sendTouchRelease(timestamp, 0, touchPoint);
310+}
311+
312 QTEST_MAIN(tst_UCSwipeArea)
313
314 #include "tst_swipearea.moc"

Subscribers

People subscribed via source and target branches