Merge lp:~tpeeters/ubuntu-ui-toolkit/panel-hideTimer into lp:ubuntu-ui-toolkit

[Tim Peeters]

Reset hide timer when user interacts with toolbar or toolbar buttons.

Move the hide timer from Toolbar to Panel, trigger it when open() is called, and restart it on user interaction.

The timer was moved because open() is defined in Panel, and user interaction (pressing and releasing on top of the panel) are dealt with in Panel, not Toolbar, and open() and user interactions need to reset the timer. To keep backwards UX compatibility for Panel, the default hideTimeout was set to -1 ==> no timer.

[lp:~tpeeters/ubuntu-ui-toolkit/panel-hideTimer]

887. By Tim Peeters on 2013-12-02

fix conditional restart

888. By Tim Peeters on 2013-12-02

clean

889. By Tim Peeters on 2013-12-02

update panel unit tests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:889
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1377/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1366
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1339
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/325
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/325/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1216
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1366
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1366/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1339
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1339/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3878
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2001

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1377/rebuild

review: Approve (continuous-integration)

[Tim Peeters (tpeeters)]

Tested gallery-app and webbrowser-app on maguro, and executed ubuntuuitoolkit AP tests on same device. All good.

[Tim Peeters (tpeeters)]

Webbrowser AP tests also pass on device

[Cris Dywan (kalikiana)]

What's the motivation behind these numbers? "wait(3*panel.hideTimeout/4);"

review: Needs Information

[lp:~tpeeters/ubuntu-ui-toolkit/panel-hideTimer]

890. By Tim Peeters on 2013-12-03

improved test and explained it better

891. By Tim Peeters on 2013-12-03

improve toolbar tests

[Tim Peeters (tpeeters)]

> What's the motivation behind these numbers? "wait(3*panel.hideTimeout/4);"

I simplified it to *0.6, and added explanation in the code.
Basically, in total you want to wait > 1.0*hideTimeout, but with user interaction inbetween, so you wait twice < hideTimeout, and you check that because of the user interaction the toolbar/panel is not closed.

[Cris Dywan (kalikiana)]

That makes sense. I guessed it would be something like that but it's good to be clear so everyone doesn't have to ponder what it does. Thanks for adding the explanations.

> + compare(panel.opened, false, "Toolbar automatically closes after timeout");
s/Toolbar/Panel
> + the panel frmo detecting interaction and the timer will not be reset.
s/frmo/from

review: Needs Fixing

[lp:~tpeeters/ubuntu-ui-toolkit/panel-hideTimer]

892. By Tim Peeters on 2013-12-03

fix typos

[Cris Dywan (kalikiana)]

Nice!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:891
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1391/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1395
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1366/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/339
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/339
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/339/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1237
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1395
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1395/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1366
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1366/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3900/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2024

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1391/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'CHANGES'
--- CHANGES	2013-11-28 15:07:31 +0000
+++ CHANGES	2013-12-03 18:38:40 +0000
@@ -79,6 +79,7 @@
 * CHANGED IN Header: property Component contents TO property Item contents
 * ADDED IN Panel: function open()
 * ADDED IN Panel: function close()
+* ADDED IN Panel: property int hideTimeout
 * DEPRECATED IN Panel: writable property opened. Will be made read-only.
 * ADDED IN: Empty: property bool waitingConfirmationForRemoval
 * ADDED IN: Empty: function cancelItemRemoval()

=== modified file 'components.api'
--- components.api	2013-12-02 19:14:03 +0000
+++ components.api	2013-12-03 18:38:40 +0000
@@ -234,6 +234,7 @@
     property bool opened
     function open()
     function close()
+    property int hideTimeout
     property bool locked
     property real hintSize
     property real triggerSize

=== modified file 'modules/Ubuntu/Components/Panel.qml'
--- modules/Ubuntu/Components/Panel.qml	2013-11-27 14:54:05 +0000
+++ modules/Ubuntu/Components/Panel.qml	2013-12-03 18:38:40 +0000
@@ -103,8 +103,7 @@
     Any Items can be placed inside the Panel, but MouseAreas can block mouse events from reaching
     the panel and thus obstruct the swiping behavior for hiding the panel. As a result, the user cannot
     start swiping on the buttons in the examples above in order to hide the panel. To remedy this, clicked()
-    signals are forwarded from the panel to its children. The children's clicked() signal does not have
-    a mouse parameter. Example:
+    signals are forwarded from the panel by calling the child's trigger() function. Example:
     \qml
         import QtQuick 2.0
         import Ubuntu.Components 0.1
@@ -130,15 +129,17 @@
                         width: units.gu(8)
                         height: units.gu(4)
                         anchors.centerIn: parent
-                        color: Theme.palette.normal.foreground
-                        signal clicked()
-                        onClicked: print("The red rectangle was clicked");
+                        color: "red"
+                        function trigger() {
+                            print("The red rectangle was clicked");
+                        }
                     }
                 }
             }
+            Component.onCompleted: panel.open();
         }
     \endqml
-    Like this, the red rectangle accepts clicked() events, but the user can still swipe down on top
+    Like this, the red rectangle accepts click events, but the user can still swipe down on top
     of the rectangle in order to hide the panel.
 */
 Item {
@@ -208,6 +209,7 @@
         // FIXME: When opened is made readonly, openedChangedWarning must be removed
         internal.openedChangedWarning = false;
         panel.state = "spread";
+        hideTimer.conditionalRestart();
     }
 
     /*!
@@ -217,9 +219,21 @@
         // FIXME: When opened is made readonly, openedChangedWarning must be removed.
         internal.openedChangedWarning = false;
         panel.state = "";
+        hideTimer.stop();
     }
 
     /*!
+      The time in milliseconds before the panel automatically hides after inactivity
+      when it is not locked. Interacting with the panel resets the timer.
+      Note that adding contents to the panel that accepts mouse events will prevent
+      the panel from detecting interaction and the timer will not be reset.
+      Setting a negative value will disable automatic hiding.
+      Default value: -1 (automatic hiding is disabled).
+      \qmlproperty int hideTimeout
+     */
+    property alias hideTimeout: hideTimer.interval
+
+    /*!
       Disable edge swipe to open/close the panel. False by default.
      */
     property bool locked: false
@@ -228,6 +242,35 @@
         if (state == "hint" || state == "moving") {
             draggingArea.finishMoving();
         }
+        if (!hideTimer.conditionalRestart()) {
+            hideTimer.stop();
+        }
+    }
+
+    Timer {
+        id: hideTimer
+        interval: -1
+        running: panel.opened && !panel.locked && interval >= 0
+
+        function conditionalRestart() {
+            if (hideTimer.interval >= 0) {
+                if (!panel.locked && panel.opened) {
+                    hideTimer.restart();
+                    return true;
+                }
+            }
+            return false;
+        }
+        onIntervalChanged: {
+            if (!conditionalRestart()) {
+                hideTimer.stop();
+            }
+        }
+        onTriggered: {
+            if (!panel.locked) {
+                panel.close();
+            }
+        }
     }
 
     /*!
@@ -485,6 +528,7 @@
 
         property int initialPosition
         onPressed: {
+            hideTimer.stop();
             pressedItem = getTriggerableItem(mouse);
             if (panel.locked) return;
             initialPosition = getMousePosition();
@@ -514,12 +558,16 @@
         onReleased: {
             if (panel.state == "moving" || panel.state == "hint") {
                 finishMoving();
+            } else {
+                hideTimer.conditionalRestart();
             }
         }
         // Mouse cursor moving out of the window while pressed on desktop
         onCanceled: {
             if (panel.state == "moving" || panel.state == "hint") {
                 finishMoving();
+            } else {
+                hideTimer.conditionalRestart();
             }
         }
 

=== modified file 'modules/Ubuntu/Components/Toolbar.qml'
--- modules/Ubuntu/Components/Toolbar.qml	2013-11-08 18:38:25 +0000
+++ modules/Ubuntu/Components/Toolbar.qml	2013-12-03 18:38:40 +0000
@@ -42,12 +42,7 @@
      */
     property Item tools: null
 
-    /*!
-      \preliminary
-      The time in milliseconds before the toolbar automatically hides after inactivity
-      when it is not locked.
-     */
-    property int hideTimeout: 5000
+    hideTimeout: 5000
 
     /*! \internal */
     onToolsChanged: {
@@ -66,10 +61,6 @@
             if (tools && tools.hasOwnProperty("opened")) {
                 tools.opened = toolbar.opened;
             }
-
-            if (!toolbar.locked) {
-                hideTimer.restart();
-            }
         } else { // no tools
             locked = true;
             toolbar.close();
@@ -79,22 +70,10 @@
     // if tools is not specified, lock the toolbar in closed position
     locked: tools && tools.hasOwnProperty("locked") ? tools.locked : false
 
-    Timer {
-        id: hideTimer
-        interval: toolbar.hideTimeout
-        running: toolbar.opened && !toolbar.locked
-        onTriggered: {
-            if (!toolbar.locked) {
-                toolbar.close();
-            }
-        }
-    }
-
     onOpenedChanged: {
         if (tools && tools.hasOwnProperty("opened")) {
             tools.opened = toolbar.opened;
         }
-        if (!toolbar.locked) hideTimer.restart();
     }
 
     Connections {

=== modified file 'tests/unit_x11/tst_components/tst_panel.qml'
--- tests/unit_x11/tst_components/tst_panel.qml	2013-09-09 18:59:35 +0000
+++ tests/unit_x11/tst_components/tst_panel.qml	2013-12-03 18:38:40 +0000
@@ -31,6 +31,10 @@
             right: parent.right
         }
         height: parent.height / 2
+        Rectangle {
+            color: "pink"
+            anchors.fill: parent
+        }
     }
 
     TestCase {
@@ -162,6 +166,35 @@
             panel.align = Qt.AlignBottom;
         }
 
+        function test_clickToDeactivate() {
+            panel.open();
+            compare(panel.opened && panel.align === Qt.AlignBottom, true, "Panel is opened and bottom-aligned");
+            mouseClick(root, root.width / 2, 5, Qt.LeftButton);
+            compare(panel.opened, false, "Panel is deactivated by clicking in the view outside of the panel");
+        }
+
+        function test_hideTimeout_bug1249031() {
+            compare(panel.hideTimeout, -1, "Panel hide timeout is initially negative (no timeout)");
+            panel.hideTimeout = 2000;
+            panel.open();
+            compare(panel.opened, true, "Panel can be made opened");
+            wait(panel.hideTimeout + 500); // add 500 ms margin
+            compare(panel.opened, false, "Panel automatically closes after timeout");
+
+            // now, wait in total more than hideTimeout, but less than 2*hideTimeout,
+            //  and have user interaction half-way to verify that the interaction
+            //  resets the timer and the panel is not closed.
+            panel.open();
+            wait(0.6*panel.hideTimeout);
+            mouseClick(panel, panel.width/2, panel.height/2);
+            wait(0.6*panel.hideTimeout);
+            compare(panel.opened, true, "Interacting with panel contents resets the hide timer");
+            // verify that the timer is still running by waiting a bit longer:
+            wait(0.6*panel.hideTimeout);
+            compare(panel.opened, false, "Interacting with the panel contents does not stop the timer")
+            panel.hideTimeout = -1;
+        }
+
         QtObject {
             id: swipeTests
 
@@ -239,13 +272,6 @@
                 testCase.mouseRelease(panel, x - dx, y - dy, Qt.LeftButton);
                 testCase.compare(panel.opened, false, "Top-aligned panel deactivated by swiping up (delay: "+moveDelay+"");
             }
-
-            function test_clickToDeactivate() {
-                panel.open();
-                compare(panel.opened && panel.align === Qt.AlignBottom, true, "Panel is opened and bottom-aligned");
-                mouseClick(root, root.width / 2, 5, Qt.LeftButton);
-                compare(panel.opened, false, "Panel is deactivated by clicking in the view outside of the panel");
-            }
         }
     }
 }

=== modified file 'tests/unit_x11/tst_components/tst_toolbar.qml'
--- tests/unit_x11/tst_components/tst_toolbar.qml	2013-11-08 18:32:39 +0000
+++ tests/unit_x11/tst_components/tst_toolbar.qml	2013-12-03 18:38:40 +0000
@@ -36,6 +36,7 @@
             tools: ToolbarItems {
                 id: toolbarItems
                 ToolbarButton {
+                    id: toolbarButton
                     text: "action1"
                 }
             }
@@ -74,12 +75,24 @@
             compare(mainView.__propagated.toolbar.opened, false, "Toolbar can be made closed by setting page.tools.opened to false");
         }
 
-        function test_hideTimeout() {
+        function test_hideTimeout_bug1249031() {
             compare(mainView.__propagated.toolbar.hideTimeout, 5000, "Toolbar hide timeout is initially 5 seconds.");
             mainView.__propagated.toolbar.open();
             compare(mainView.__propagated.toolbar.opened, true, "Toolbar can be made opened");
             wait(mainView.__propagated.toolbar.hideTimeout + 500); // add 500 ms margin
             compare(mainView.__propagated.toolbar.opened, false, "Toolbar automatically closes after timeout");
+
+            // now, wait in total more than hideTimeout, but less than 2*hideTimeout,
+            //  and have user interaction half-way to verify that the interaction
+            //  resets the timer and the toolbar is not closed.
+            mainView.__propagated.toolbar.open();
+            wait(0.6*mainView.__propagated.toolbar.hideTimeout);
+            mouseClick(toolbarButton, toolbarButton.width/2, toolbarButton.height/2);
+            wait(0.6*mainView.__propagated.toolbar.hideTimeout);
+            compare(mainView.__propagated.toolbar.opened, true, "Interacting with toolbar contents resets the hide timer");
+            // verify that the timer is still running by waiting a bit longer:
+            wait(0.6*mainView.__propagated.toolbar.hideTimeout);
+            compare(mainView.__propagated.toolbar.opened, false, "Interacting with the toolbar contents does not stop the timer")
         }
 
         function test_locked() {