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

[Zsombor Egri]

AbstractButton.pressAndHold captured in Connection.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1662
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2338/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1066
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1068
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1068/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1065

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

review: Needs Fixing (continuous-integration)

[lp:~zsombi/ubuntu-ui-toolkit/abstractbutton_longpress]

1663. By Zsombor Egri on 2015-09-30

fix

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1663
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2344/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1072
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1074
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1074/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1071

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

review: Approve (continuous-integration)

[Christian Dywan (kalikiana)]

Looks clean and sensible.

Idly wondering if the current uses of UniqueConnection should be re-evaluated as you're avoiding it here for performance reasons and we do use it in ucqquickimageextension and the QML plugin right now/

review: Approve

[Zsombor Egri (zsombi)]

> Looks clean and sensible.
>
> Idly wondering if the current uses of UniqueConnection should be re-evaluated
> as you're avoiding it here for performance reasons and we do use it in
> ucqquickimageextension and the QML plugin right now/

Yes, we should! Especially in the image, as that is pretty slow right now!

=== modified file 'src/Ubuntu/Components/plugin/ucabstractbutton.cpp'
--- src/Ubuntu/Components/plugin/ucabstractbutton.cpp	2015-08-25 11:31:29 +0000
+++ src/Ubuntu/Components/plugin/ucabstractbutton.cpp	2015-09-30 18:55:30 +0000
@@ -52,15 +52,14 @@
     : UCActionItem(parent)
     , m_mouseArea(new QQuickMouseArea)
     , m_acceptEvents(true)
+    , m_pressAndHoldConnected(false)
 {
     setActiveFocusOnPress(true);
 }
 
 bool UCAbstractButton::isPressAndHoldConnected()
 {
-    static QMetaMethod method = QMetaMethod::fromSignal(&UCAbstractButton::pressAndHold);
-    static int signalIdx = QMetaObjectPrivate::signalIndex(method);
-    return QObjectPrivate::get(this)->isSignalConnected(signalIdx);
+    IS_SIGNAL_CONNECTED(this, UCAbstractButton, pressAndHold, ());
 }
 
 void UCAbstractButton::classBegin()
@@ -88,8 +87,23 @@
     connect(m_mouseArea, &QQuickMouseArea::pressedChanged, this, &UCAbstractButton::pressedChanged);
     connect(m_mouseArea, &QQuickMouseArea::hoveredChanged, this, &UCAbstractButton::hoveredChanged);
     connect(m_mouseArea, SIGNAL(clicked(QQuickMouseEvent*)), this, SLOT(_q_mouseAreaClicked()));
-    if (isPressAndHoldConnected()) {
-        connect(m_mouseArea, SIGNAL(pressAndHold(QQuickMouseEvent*)), this, SLOT(_q_mouseAreaPressAndHold()));
+    connect(m_mouseArea, SIGNAL(pressed(QQuickMouseEvent*)), this, SLOT(_q_mouseAreaPressed()));
+}
+
+// check the pressAndHold connection on runtime, as Connections
+// may not be available on compoennt completion
+void UCAbstractButton::_q_mouseAreaPressed()
+{
+    bool longPressConnected = isPressAndHoldConnected();
+    if (longPressConnected && !m_pressAndHoldConnected) {
+        // do not use UniqueConnection as that has huge impact on performance
+        connect(m_mouseArea, SIGNAL(pressAndHold(QQuickMouseEvent*)),
+                this, SLOT(_q_mouseAreaPressAndHold()));
+        m_pressAndHoldConnected = true;
+    } else if (!longPressConnected && m_pressAndHoldConnected) {
+        disconnect(m_mouseArea, SIGNAL(pressAndHold(QQuickMouseEvent*)),
+                   this, SLOT(_q_mouseAreaPressAndHold()));
+        m_pressAndHoldConnected = false;
     }
 }
 

=== modified file 'src/Ubuntu/Components/plugin/ucabstractbutton.h'
--- src/Ubuntu/Components/plugin/ucabstractbutton.h	2015-08-20 15:33:51 +0000
+++ src/Ubuntu/Components/plugin/ucabstractbutton.h	2015-09-30 18:55:30 +0000
@@ -20,6 +20,7 @@
 #include "ucactionitem.h"
 
 class QQuickMouseArea;
+class QQuickMouseEvent;
 class UCAbstractButton : public UCActionItem
 {
     Q_OBJECT
@@ -51,12 +52,14 @@
     void pressAndHold();
 
 protected Q_SLOTS:
+    void _q_mouseAreaPressed();
     void _q_mouseAreaClicked();
     void _q_mouseAreaPressAndHold();
 
 protected:
     QQuickMouseArea *m_mouseArea;
     bool m_acceptEvents:1;
+    bool m_pressAndHoldConnected:1;
 
     bool isPressAndHoldConnected();
 };

=== modified file 'tests/unit_x11/tst_components/tst_abstractbutton13.qml'
--- tests/unit_x11/tst_components/tst_abstractbutton13.qml	2015-08-25 11:31:29 +0000
+++ tests/unit_x11/tst_components/tst_abstractbutton13.qml	2015-09-30 18:55:30 +0000
@@ -41,6 +41,21 @@
             height: width
             function trigger() {}
         }
+        Loader {
+            id: loader
+            width: units.gu(10)
+            height: units.gu(10)
+            sourceComponent: AbstractButton { objectName: "dynamic"}
+            property bool click: false
+            property bool longPress: false
+        }
+    }
+
+    Connections {
+        id: test
+        target: loader.item
+        onClicked: loader.click = true
+        onPressAndHold: loader.longPress = true
     }
 
     Action {
@@ -73,6 +88,8 @@
         function cleanup() {
             signalSpy.clear();
             triggeredSpy.clear();
+            loader.click = false;
+            loader.longPress = false;
         }
 
         function test_action() {
@@ -111,5 +128,16 @@
             mouseRelease(absLongTap, centerOf(absLongTap).x, centerOf(absLongTap).y);
             compare(signalSpy.count, 0, "click() must be suppressed when pressAndHold handler is implemented");
         }
+
+        function test_pressAndHold_emitted_on_connections_bug1495554() {
+            mouseLongPress(loader.item, centerOf(loader.item).x, centerOf(loader.item).y);
+            mouseRelease(loader.item, centerOf(loader.item).x, centerOf(loader.item).y);
+            compare(loader.click, false, "clicked should not be emitted");
+            compare(loader.longPress, true, "pressAndHold not captured by Connection");
+        }
+        function test_clicked_emitted_on_connections_bug1495554() {
+            mouseClick(loader.item, centerOf(loader.item).x, centerOf(loader.item).y);
+            compare(loader.click, true, "clicked not captured by Connection");
+        }
     }
 }