Merge lp:~aacid/ubuntu-ui-toolkit/swipeAreaPressedSignal into lp:ubuntu-ui-toolkit/staging

[Albert Astals Cid]

Simplify logic for emitting draggingChanged/pressedChanged

No need for a switch that considers the possible cases, just store the status before and check after

Fixes an issue in which the SwipeArea pressedChanged signal was not emitted if switching
directly from WaitingForTouch to Recognized

[Daniel d'Andrada (dandrader)]

Code looks ok, but it needs a test

[lp:~aacid/ubuntu-ui-toolkit/swipeAreaPressedSignal]

1734. By Albert Astals Cid on 2015-12-07

Add a test for the pressed signal emission fix

[Daniel d'Andrada (dandrader)]

Code looks good to me (didn't test it myself though).

review: Approve (code)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1733
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2569/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1297
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1299
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1299/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1296

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2569/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1734
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2574/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1302
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1304/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1301

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2574/rebuild

review: Needs Fixing (continuous-integration)

[Tim Peeters (tpeeters)]

Can you add --wipe to the usage doc in the h) case?

[Tim Peeters (tpeeters)]

sorry, my previous comment was meant for another MR.

[Christian Dywan (kalikiana)]

The change looks sensible, and the test as well.

You seem to be hitting unrelated CI problems from the looks of it, which don't come from this change. As we are in the middle of looking into uncovering some test issues you may want to merge staging and see if it improves - if not, I'll take another look.

[Albert Astals Cid (aacid)]

> The change looks sensible, and the test as well.
>
> You seem to be hitting unrelated CI problems from the looks of it, which don't
> come from this change. As we are in the middle of looking into uncovering some
> test issues you may want to merge staging and see if it improves - if not,
> I'll take another look.

There's nothing in staging to merge as far as i can see.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/Ubuntu/Components/plugin/gestures/ucswipearea.cpp'
--- src/Ubuntu/Components/plugin/gestures/ucswipearea.cpp	2015-11-13 08:15:59 +0000
+++ src/Ubuntu/Components/plugin/gestures/ucswipearea.cpp	2015-12-07 14:41:04 +0000
@@ -714,29 +714,26 @@
         recognitionTimer->stop();
     }
 
+    const bool wasDragging = q->dragging();
+    const bool wasPressed = q->pressed();
+
     status = newStatus;
     Q_EMIT statusChanged(status);
 
     SA_TRACE(statusToString(oldStatus) << " -> " << statusToString(newStatus));
 
-    switch (newStatus) {
-        case WaitingForTouch:
-            if (oldStatus == Recognized) {
-                Q_EMIT q->draggingChanged(false);
-            }
-            Q_EMIT q->pressedChanged(false);
-            break;
-        case Undecided:
-            recognitionTimer->start();
-            Q_EMIT q->pressedChanged(true);
-            break;
-        case Recognized:
-            Q_EMIT q->draggingChanged(true);
-            break;
-        default:
-            // no-op
-            break;
+    if (newStatus == Undecided) {
+        recognitionTimer->start();
     }
+
+    const bool isDragging = q->dragging();
+    const bool isPressed = q->pressed();
+
+    if (isDragging != wasDragging)
+        Q_EMIT q->draggingChanged(isDragging);
+
+    if (isPressed != wasPressed)
+        Q_EMIT q->pressedChanged(isPressed);
 }
 
 void UCSwipeAreaPrivate::updatePosition(const QPointF &point)

=== modified file 'tests/unit_x11/tst_swipearea/tst_swipearea.cpp'
--- tests/unit_x11/tst_swipearea/tst_swipearea.cpp	2015-11-17 12:20:03 +0000
+++ tests/unit_x11/tst_swipearea/tst_swipearea.cpp	2015-12-07 14:41:04 +0000
@@ -884,11 +884,18 @@
 
     QPoint touch0Pos(edgeDragArea->width()/2.0f, m_view->height()/2.0f);
 
+    QSignalSpy pressedSpy(edgeDragArea, &UCSwipeArea::pressedChanged);
+    QCOMPARE(edgeDragArea->pressed(), false);
+
     QTest::touchEvent(m_view, m_device).press(0, touch0Pos);
 
     // check for immediate recognition
     QCOMPARE((int)edgeDragArea->d->status, (int)UCSwipeAreaPrivate::Recognized);
 
+    // and it is pressed
+    QCOMPARE(edgeDragArea->pressed(), true);
+    QCOMPARE(pressedSpy.count(), 1);
+
     // and therefore it should have immediately grabbed the touch point,
     // not letting it leak to items behind him.
     QCOMPARE(dummyItem->touchEvents.count(), 0);