Merge lp:~tpeeters/ubuntu-ui-toolkit/20-AppHeader-use-cppHeader into lp:ubuntu-ui-toolkit/staging

[Tim Peeters]

Use the new cpp Header in the internal AppHeader.

[lp:~tpeeters/ubuntu-ui-toolkit/20-AppHeader-use-cppHeader]

1669. By Tim Peeters on 2015-10-01

clean MainView.qml

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1668
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2353/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1081
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1083
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1083/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1080

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1669
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2354/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1082
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1084
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1084/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1081

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

review: Needs Fixing (continuous-integration)

[lp:~tpeeters/ubuntu-ui-toolkit/20-AppHeader-use-cppHeader]

1670. By Tim Peeters on 2015-10-01

sync staging

1671. By Tim Peeters on 2015-10-01

fix page positioning with locked header

1672. By Tim Peeters on 2015-10-01

restore contentY value when unsetting flickable to pass test_flickableY_bug1201452() of tst_page13

1673. By Tim Peeters on 2015-10-01

update header flickable when page flickable is updated.

1674. By Tim Peeters on 2015-10-01

clean

1675. By Tim Peeters on 2015-10-01

better contentY computation

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1675
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2356/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1084
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1086
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1086/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1083

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

review: Approve (continuous-integration)

[Zsombor Egri (zsombi)]

No complains! Starts to shape up nicely!

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'components.api'
--- components.api	2015-10-01 12:49:42 +0000
+++ components.api	2015-10-01 21:23:26 +0000
@@ -687,6 +687,7 @@
     readonly property Action actions
     property Action backAction
     property Item contents
+    property Flickable flickable
     property color foregroundColor
     property bool locked
     property string preset

=== modified file 'src/Ubuntu/Components/1.3/AppHeader.qml'
--- src/Ubuntu/Components/1.3/AppHeader.qml	2015-09-03 13:27:13 +0000
+++ src/Ubuntu/Components/1.3/AppHeader.qml	2015-10-01 21:23:26 +0000
@@ -23,19 +23,13 @@
     \inqmlmodule Ubuntu.Components 1.1
     \ingroup ubuntu
 */
-Components.StyledItem {
+Components.Header {
     id: header
 
     anchors {
         left: parent.left
         right: parent.right
     }
-    y: 0
-
-    /*!
-      Animate showing and hiding of the header.
-     */
-    property bool animate: true
 
     /*!
       The background color of the divider. Value set by MainView.
@@ -47,79 +41,19 @@
      */
     property color panelColor
 
-    Behavior on y {
-        enabled: animate && !(header.flickable && header.flickable.moving)
-        SmoothedAnimation {
-            duration: Components.UbuntuAnimation.BriskDuration
-        }
-    }
-
-    /*! \internal */
-    onHeightChanged: {
-        internal.checkFlickableMargins();
-        internal.movementEnded();
-        if (header.config.visible) {
-            header.show();
-        } else {
-            header.hide();
-        }
-    }
-
-    // with PageHeadConfiguration 1.2, always be visible.
-    visible: title || contents || tabsModel || internal.newConfig
-    onVisibleChanged: {
-        internal.checkFlickableMargins();
-    }
-    enabled: header.y === 0
-
-    /*!
-      Show the header
-     */
-    function show() {
-        if (internal.newConfig) {
-            header.config.visible = true;
-        }
-        // Enable the header as soon as it finished animating
-        //  to the fully visible state:
-        header.enabled = Qt.binding(function() { return header.y === 0; });
-        header.y = 0;
-    }
-
-    /*!
-      Hide the header
-     */
-    function hide() {
-        if (internal.newConfig) {
-            header.config.visible = false;
-        }
-        // Disable the header immediately (the update of the y-value
-        //  is delayed because of the Behavior defined on it):
-        header.enabled = false;
-        header.y = -header.height;
-    }
+    // prevent triggering buttons in the header when it is moving
+    enabled: header.exposed && !header.moving
 
     /*!
       The text to display in the header
      */
     property string title: ""
-    onTitleChanged: {
-        // deprecated for new versions of PageHeadConfiguration
-        if (!internal.newConfig) {
-            header.show();
-        }
-    }
 
     /*!
       The contents of the header. If this is set, \l title will be ignored.
       DEPRECATED and replaced by Page.head.contents.
      */
     property Item contents: null
-    onContentsChanged: {
-        // deprecated for new versions of PageHeadConfiguration
-        if (!internal.newConfig) {
-            header.show();
-        }
-    }
 
     /*!
       A model of tabs to represent in the header.
@@ -171,19 +105,6 @@
     }
 
     /*!
-      The flickable that controls the movement of the header.
-      Will be set automatically by Pages inside a MainView, but can
-      be overridden.
-     */
-    property Flickable flickable: null
-    onFlickableChanged: {
-        internal.connectFlickable();
-        if (!internal.newConfig || !header.config.locked) {
-            header.show();
-        }
-    }
-
-    /*!
       Configuration of the header.
       FIXME: Must be of type PageHeadConfiguration. Setting that as the property type
       however will use the latest version (1.3) and a Page that uses an older
@@ -191,160 +112,41 @@
      */
     property QtObject config: null
     onConfigChanged: {
-        // set internal.newConfig because when we rely on the binding,
-        //  the value of newConfig may be updated after executing the code below.
-        internal.newConfig = config && config.hasOwnProperty("visible") &&
-                config.hasOwnProperty("locked");
-        internal.connectFlickable();
+        if (header.config.locked) {
+            header.flickable = null;
+        } else {
+            header.flickable = header.config.flickable;
+        }
 
-        if (internal.newConfig && header.config.locked &&!header.config.visible) {
-            header.hide();
+        if (!header.flickable && !header.config.visible) {
+            // locked.
+            header.exposed = false;
         } else {
-            header.show();
+            header.config.visible = true;
+            header.exposed = true;
+        }
+    }
+    onExposedChanged: {
+        if(header.config) {
+            header.config.visible = exposed;
         }
     }
     Connections {
         target: header.config
-        ignoreUnknownSignals: true // PageHeadConfiguration <1.2 lacks the signals below
+        ignoreUnknownSignals: true
         onVisibleChanged: {
-            if (header.config.visible) {
-                header.show();
+            header.exposed = header.config.visible;
+        }
+        onLockedChanged: {
+            if (header.config.locked) {
+                header.flickable = null;
             } else {
-                header.hide();
+                header.flickable = header.config.flickable;
             }
-            internal.checkFlickableMargins();
         }
-        onLockedChanged: {
-            internal.connectFlickable();
+        onFlickableChanged: {
             if (!header.config.locked) {
-                internal.movementEnded();
-            }
-        }
-    }
-
-    /*!
-      The header is not fully opened or fully closed.
-
-      This property is true if the header is animating towards a fully
-      opened or fully closed state, or if the header is moving due to user
-      interaction with the flickable.
-
-      The value of moving is always false when using an old version of
-      PageHeadConfiguration (which does not have the visible property).
-
-      Used in tst_header_locked_visible.qml.
-    */
-    readonly property bool moving: internal.newConfig &&
-                                   ((config.visible && header.y !== 0) ||
-                                    (!config.visible && header.y !== -header.height))
-
-    QtObject {
-        id: internal
-
-        // This property is updated in header.onConfigChanged to ensure it
-        //  is updated before other functions are called in onConfigChanged.
-        property bool newConfig: header.config &&
-                                 header.config.hasOwnProperty("locked") &&
-                                 header.config.hasOwnProperty("visible")
-
-        /*!
-          Track the y-position inside the flickable.
-         */
-        property real previousContentY: 0
-
-        /*!
-          The previous flickable to disconnect events
-         */
-        property Flickable previousFlickable: null
-
-        /*!
-          Disconnect previous flickable, and connect the new one.
-         */
-        function connectFlickable() {
-            // Finish the current header movement in case the current
-            // flickable is disconnected while scrolling:
-            internal.movementEnded();
-
-            if (previousFlickable) {
-                previousFlickable.contentYChanged.disconnect(internal.scrollContents);
-                previousFlickable.movementEnded.disconnect(internal.movementEnded);
-                previousFlickable.interactiveChanged.disconnect(internal.interactiveChanged);
-                previousFlickable = null;
-            }
-            if (flickable && !(internal.newConfig && header.config.locked)) {
-                // Connect flicking to movements of the header
-                previousContentY = flickable.contentY;
-                flickable.contentYChanged.connect(internal.scrollContents);
-                flickable.movementEnded.connect(internal.movementEnded);
-                flickable.interactiveChanged.connect(internal.interactiveChanged);
-                flickable.contentHeightChanged.connect(internal.contentHeightChanged);
-                previousFlickable = flickable;
-            }
-            internal.checkFlickableMargins();
-        }
-
-        /*!
-          Update the position of the header to scroll with the flickable.
-         */
-        function scrollContents() {
-            // Avoid updating header.y when rebounding or being dragged over the bounds.
-            if (!flickable.atYBeginning && !flickable.atYEnd) {
-                var deltaContentY = flickable.contentY - previousContentY;
-                // FIXME: MathUtils.clamp  is expensive. Fix clamp, or replace it here.
-                header.y = MathUtils.clamp(header.y - deltaContentY, -header.height, 0);
-            }
-            previousContentY = flickable.contentY;
-        }
-
-        /*!
-          Fully show or hide the header, depending on its current y.
-         */
-        function movementEnded() {
-            if (!(internal.newConfig && header.config.locked)) {
-                if ( (flickable && flickable.contentY < 0) ||
-                        (header.y > -header.height/2)) {
-                    header.show();
-                } else {
-                    header.hide();
-                }
-            }
-        }
-
-        /*
-          Content height of flickable changed
-         */
-        function contentHeightChanged() {
-            if (flickable && flickable.height >= flickable.contentHeight) header.show();
-        }
-
-        /*
-          Flickable became interactive or non-interactive.
-         */
-        function interactiveChanged() {
-            if (flickable && !flickable.interactive) header.show();
-        }
-
-        /*
-          Check the topMargin of the flickable and set it if needed to avoid
-          contents becoming unavailable behind the header.
-         */
-        function checkFlickableMargins() {
-            if (header.flickable) {
-                var headerHeight = 0;
-                if (header.visible && !(internal.newConfig &&
-                                        header.config.locked &&
-                                        !header.config.visible)) {
-                    headerHeight = header.height;
-                }
-
-                if (flickable.topMargin !== headerHeight) {
-                    var oldContentY = flickable.contentY;
-                    var previousHeaderHeight = flickable.topMargin;
-                    flickable.topMargin = headerHeight;
-                    // push down contents when header grows,
-                    // pull up contents when header shrinks.
-                    flickable.contentY = oldContentY - headerHeight + previousHeaderHeight;
-                }
+                header.flickable = header.config.flickable;
             }
         }
     }

=== modified file 'src/Ubuntu/Components/1.3/MainView.qml'
--- src/Ubuntu/Components/1.3/MainView.qml	2015-09-29 14:15:35 +0000
+++ src/Ubuntu/Components/1.3/MainView.qml	2015-10-01 21:23:26 +0000
@@ -161,7 +161,6 @@
             panelColor: Qt.lighter(mainView.headerColor, 1.1)
 
             title: internal.activePage ? internal.activePage.title : ""
-            flickable: internal.activePage ? internal.activePage.flickable : null
             pageStack: internal.activePage ? internal.activePage.pageStack : null
 
             contents: internal.activePage &&
@@ -199,7 +198,7 @@
                     if (!(headerItem.config &&
                           headerItem.config.hasOwnProperty("locked") &&
                           headerItem.config.locked)) {
-                        headerItem.show();
+                        headerItem.exposed = true;
                     }
                 }
             }

=== modified file 'src/Ubuntu/Components/1.3/Page.qml'
--- src/Ubuntu/Components/1.3/Page.qml	2015-07-03 14:53:08 +0000
+++ src/Ubuntu/Components/1.3/Page.qml	2015-10-01 21:23:26 +0000
@@ -44,6 +44,7 @@
     Toolkit13.PageHeadConfiguration {
         id: headerConfig
         title: page.title
+        flickable: page.flickable
     }
 
     Toolkit13.Object {

=== modified file 'src/Ubuntu/Components/1.3/PageHeadConfiguration.qml'
--- src/Ubuntu/Components/1.3/PageHeadConfiguration.qml	2015-06-15 07:45:34 +0000
+++ src/Ubuntu/Components/1.3/PageHeadConfiguration.qml	2015-10-01 21:23:26 +0000
@@ -55,7 +55,8 @@
     property bool locked: false
 
     // auto-updated by AppHeader, but may be set by the developer
-    property bool visible
+    property bool visible: true
 
     property string title
+    property Flickable flickable
 }

=== modified file 'src/Ubuntu/Components/plugin/ucheader.cpp'
--- src/Ubuntu/Components/plugin/ucheader.cpp	2015-09-23 11:35:33 +0000
+++ src/Ubuntu/Components/plugin/ucheader.cpp	2015-10-01 21:23:26 +0000
@@ -111,9 +111,12 @@
     if (m_exposed || (!m_flickable.isNull() && m_flickable->contentY() <= 0.0)) {
         // Header was exposed before, or the flickable is scrolled up close to
         //  the top so that the header should be visible.
-        show();
+        // After Header completed, the theming engine updates its implicitHeight
+        //  to that of the style and we do not want to animate then. Other height
+        //  changes also do not need animations.
+        show(false);
     } else {
-        hide();
+        hide(false);
     }
 }
 
@@ -151,11 +154,22 @@
 
         // Finish the current header movement in case the current
         //  flickable is disconnected while scrolling.
-        _q_flickableMovementEnded();
+        if (m_exposed) {
+            show(false);
+        } else {
+            hide(false);
+        }
         m_flickable->disconnect(this);
 
+        qreal oldMargin = m_flickable->topMargin();
+        // store contentY to compensate for Flickable updating the position due to margin change.
+        qreal oldContentY = m_flickable->contentY();
         delete m_flickableTopMarginBackup; // Restores previous value/binding for topMargin.
         m_flickableTopMarginBackup = Q_NULLPTR;
+
+        qreal delta = m_flickable->topMargin() - oldMargin + m_flickable->contentY() - oldContentY;
+        // revert the flickable content Y.
+        m_flickable->setContentY(m_flickable->contentY() - delta);
     }
 
     m_flickable = flickable;
@@ -164,6 +178,7 @@
     Q_ASSERT(m_flickableTopMarginBackup == Q_NULLPTR);
     if (!m_flickable.isNull()) {
         m_flickableTopMarginBackup = new PropertyChange(m_flickable, "topMargin");
+        updateFlickableMargins();
         connect(m_flickable, SIGNAL(contentYChanged()),
                 this, SLOT(_q_scrolledContents()));
         connect(m_flickable, SIGNAL(movementEnded()),
@@ -173,8 +188,7 @@
         connect(m_flickable, SIGNAL(interactiveChanged()),
                 this, SLOT(_q_flickableInteractiveChanged()));
         m_previous_contentY = m_flickable->contentY();
-        updateFlickableMargins();
-        show();
+        _q_flickableMovementEnded(); // show or hide depending on y
     }
 }
 
@@ -193,7 +207,7 @@
     }
 }
 
-void UCHeader::show() {
+void UCHeader::show(bool animate) {
     if (!m_exposed) {
         m_exposed = true;
         Q_EMIT exposedChanged();
@@ -202,11 +216,14 @@
             m_showHideAnimation->stop();
         }
     }
-    if (isComponentComplete()) {
+
+    if (animate && isComponentComplete()) {
         m_showHideAnimation->setFrom(y());
         m_showHideAnimation->setTo(0.0);
         m_showHideAnimation->start();
     } else {
+        // If a previous animation was showing the header, stop it.
+        m_showHideAnimation->stop();
         this->setY(0.0);
         if (m_moving) {
             m_moving = false;
@@ -215,7 +232,7 @@
     }
 }
 
-void UCHeader::hide() {
+void UCHeader::hide(bool animate) {
     if (m_exposed) {
         m_exposed = false;
         Q_EMIT exposedChanged();
@@ -224,11 +241,14 @@
             m_showHideAnimation->stop();
         }
     }
-    if (isComponentComplete()) {
+
+    if (animate && isComponentComplete()) {
         m_showHideAnimation->setFrom(y());
         m_showHideAnimation->setTo(-1.0*height());
         m_showHideAnimation->start();
     } else {
+        // If a previous animation was hiding the header, stop it.
+        m_showHideAnimation->stop();
         this->setY(-1.0*height());
         if (m_moving) {
             m_moving = false;

=== modified file 'src/Ubuntu/Components/plugin/ucheader.h'
--- src/Ubuntu/Components/plugin/ucheader.h	2015-09-22 18:24:27 +0000
+++ src/Ubuntu/Components/plugin/ucheader.h	2015-10-01 21:23:26 +0000
@@ -47,8 +47,8 @@
     void movingChanged();
 
 protected:
-    void show();
-    void hide();
+    void show(bool animate = true);
+    void hide(bool animate = true);
 
 private Q_SLOTS:
     void _q_scrolledContents();

=== modified file 'tests/unit_x11/tst_components/tst_header.qml'
--- tests/unit_x11/tst_components/tst_header.qml	2015-09-22 14:13:36 +0000
+++ tests/unit_x11/tst_components/tst_header.qml	2015-10-01 21:23:26 +0000
@@ -46,8 +46,13 @@
 
     Flickable {
         id: flickable
-        anchors.fill: parent
-        contentHeight: height * 2
+        anchors {
+            top: header.flickable ? parent.top : header.bottom
+            left: parent.left
+            right: parent.right
+            bottom: parent.bottom
+        }
+        contentHeight: root.height * 2
 
         Grid {
             id: switchGrid
@@ -56,7 +61,8 @@
             anchors {
                 top: parent.top
                 left: parent.left
-                margins: units.gu(5)
+                leftMargin: units.gu(5)
+                topMargin: 2*root.initialHeaderHeight
             }
             Switch {
                 id: lockedSwitch
@@ -151,6 +157,12 @@
         id: otherHeader
     }
 
+    Header {
+        id: hiddenHeader
+        exposed: false
+        height: root.initialHeaderHeight
+    }
+
     UbuntuTestCase {
         name: "Header"
         when: windowShown
@@ -201,6 +213,12 @@
             }
         }
 
+        function test_0_initially_hidden() {
+            // Don't show an animation if the header is hidden initially.
+            compare(hiddenHeader.y, -header.height,
+                    "Hidden header has wrong initial y-value.");
+        }
+
         function test_reparent_width() {
             // test initial header width:
             compare(header.parent, root);
@@ -288,7 +306,7 @@
             wait_for_exposed(true);
         }
 
-        function test_scroll_updates_visible() {
+        function test_scroll_updates_exposed() {
             scroll_down();
             wait_for_exposed(false, "Scrolling down does not hide header.");
             scroll_up();

=== modified file 'tests/unit_x11/tst_components/tst_page13.qml'
--- tests/unit_x11/tst_components/tst_page13.qml	2015-03-23 16:16:46 +0000
+++ tests/unit_x11/tst_components/tst_page13.qml	2015-10-01 21:23:26 +0000
@@ -86,7 +86,7 @@
             page.flickable = testFlickable;
             compare(page.flickable, testFlickable, "Flickable could not be set.");
             compare(header.flickable, testFlickable,
-                    "Header flickable was not update correctly.");
+                    "Header flickable was not updated correctly.");
 
             page.flickable = null;
             compare(page.flickable, null, "Flickable cannot be unset.");
@@ -113,7 +113,7 @@
             compare(page.flickable.contentY, flickableY + headerHeight,
                     "contentY was not updated properly when header was hidden.");
 
-            page.head.locked.locked = false;
+            page.head.locked = false;
             page.head.visible = true;
             waitForHeaderAnimation(mainView);
 

=== modified file 'tests/unit_x11/tst_components/tst_pagehead_visible.qml'
--- tests/unit_x11/tst_components/tst_pagehead_visible.qml	2015-09-03 11:44:38 +0000
+++ tests/unit_x11/tst_components/tst_pagehead_visible.qml	2015-10-01 21:23:26 +0000
@@ -35,10 +35,6 @@
                 id: page
                 title: "Auto-hide"
                 head {
-                    locked: lockedSwitch.checked
-                    onVisibleChanged: {
-                        visibleSwitch.checked = page.head.visible
-                    }
                     actions: Action {
                         iconName: "like"
                         text: "Like"
@@ -71,7 +67,11 @@
                         }
                         Switch {
                             id: lockedSwitch
-                            checked: false
+                            checked: page.head.locked
+                            function trigger() {
+                                page.head.locked = !page.head.locked;
+                                print("set page.head.locked to "+page.head.locked)
+                            }
                         }
                         Label {
                             text: "header locked"
@@ -79,7 +79,10 @@
                         Switch {
                             id: visibleSwitch
                             checked: page.head.visible
-                            onClicked: page.head.visible = checked
+                            function trigger() {
+                                page.head.visible = !page.head.visible;
+                                print("set page.head.visible to "+page.head.visible)
+                            }
                         }
                         Label {
                             text: "header visible"