Merge lp:~tpeeters/ubuntu-ui-toolkit/75-sectionsScroll into lp:ubuntu-ui-toolkit/staging

[Tim Peeters]

Properly disable left/right arrows on sections when at the beginning/end of the list.

[Andrea Bernabei (faenil)]

just a quick comment, but I'd have to read the logic again to see if the change makes sense.

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1874
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/416/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1380/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/416/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1874
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/192/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1381/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/192/rebuild

review: Approve (continuous-integration)

[Andrea Bernabei (faenil)]

I left more comments.

- you can avoid setting "enabled" as Icon is not a component with an active input handler anyway.

- I vote against changing the hovering logic, hoveringLeft should be true when the mouse hovers over the left arrow, even if the arrow is currently disabled, because the mouse is effectively hovering over it.

- instead of changing the semantics of hoveringLeft/Right, I think it would be better to handle this in onPressed, where you can check for hoveringLeft/Right and atXBeginning/End

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1874
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/426/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1382/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/426/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1874
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/170/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1383/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/170/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1874
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/100/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/416
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/426
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/426/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/192
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/170
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1384/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1380
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1382
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1381
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1383

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/100/rebuild

review: Approve (continuous-integration)

[Tim Peeters (tpeeters)]

> I left more comments.

I replied to the inline comments inline.
>
> - you can avoid setting "enabled" as Icon is not a component with an active
> input handler anyway.
>
> - I vote against changing the hovering logic, hoveringLeft should be true when
> the mouse hovers over the left arrow, even if the arrow is currently disabled,
> because the mouse is effectively hovering over it.

I will change the logic to only check the mouse position when the mouse is pressed. It is not needed to check it whenever the mouse moves.

>
> - instead of changing the semantics of hoveringLeft/Right, I think it would be
> better to handle this in onPressed, where you can check for hoveringLeft/Right
> and atXBeginning/End

Right :) That's what I commented below (I didn't read this yet).

[lp:~tpeeters/ubuntu-ui-toolkit/75-sectionsScroll]

1875. By Tim Peeters on 2016-03-01

improve left/right button hover detection and action

1876. By Tim Peeters on 2016-03-01

remove enabled binding

1877. By Tim Peeters on 2016-03-01

clean

1878. By Tim Peeters on 2016-03-01

comment

[Andrea Bernabei (faenil)]

thanks.

I liked the hovering logic more, but it's your choice ;)

Maybe rename pressed(mouse) to "contains(mouse)"?

[Andrea Bernabei (faenil)]

also, it's missing unit tests

[Andrea Bernabei (faenil)]

actually, no, I think the logic you changed is broken.

Not updating onPositionChanged means that you can press on right arrow, then move your mouse to the left arrow, then release, and that will trigger a click on the *right* arrow

[lp:~tpeeters/ubuntu-ui-toolkit/75-sectionsScroll]

1879. By Tim Peeters on 2016-03-01

rename pressed() to contains()

1880. By Tim Peeters on 2016-03-01

fix logic

1881. By Tim Peeters on 2016-03-01

update comment

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1875
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/197/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1403/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/197/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1875
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/421/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1404/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/421/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1875
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/175/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1405/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/175/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1875
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/431/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1406/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/431/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1875
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/103/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/421
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/431
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/431/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/197
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/175
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1407/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1404
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1406
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1403
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1405

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/103/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1881
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/422/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1408/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/422/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1881
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/198/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1409/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/198/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1881
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/432/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1410/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/432/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1881
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/176/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1411/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/176/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1881
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/104/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/422
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/432
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/432/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/198
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/176
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1412/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1408
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1410
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1409
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1411

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/104/rebuild

review: Approve (continuous-integration)

[lp:~tpeeters/ubuntu-ui-toolkit/75-sectionsScroll]

1882. By Tim Peeters on 2016-03-02

add unit test

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:1882
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/438/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1433/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/438/rebuild

review: Needs Fixing (continuous-integration)

[Andrea Bernabei (faenil)]

commented, it would be good to also have a test for press + release on the other icon, since that was a bug that was about to get released :)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1882
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/428/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1436/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/428/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1882
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/205/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1437/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/205/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1882
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/182/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1441/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/182/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:1882
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/109/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/428
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/438/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/205
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/182
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1442/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1436
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1433
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1437
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1441

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/109/rebuild

review: Needs Fixing (continuous-integration)

[lp:~tpeeters/ubuntu-ui-toolkit/75-sectionsScroll]

1883. By Tim Peeters on 2016-03-02

add unit test

1884. By Tim Peeters on 2016-03-02

add another unit test

[Andrea Bernabei (faenil)]

lgtm! thanks :)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/429/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1443/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/429/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/206/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1444/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/206/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/430/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1445/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/430/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/207/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1446/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/207/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/439/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1447/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/439/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/183/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1448/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/183/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/110/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/429
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/439
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/439/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/206
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/183/console
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1449/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1443
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1447
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1444
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1448

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/110/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/440/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1450/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/440/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:1884
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/184/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1451/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/184/rebuild

review: Approve (continuous-integration)

=== modified file 'src/Ubuntu/Components/Themes/Ambiance/1.3/SectionsStyle.qml'
--- src/Ubuntu/Components/Themes/Ambiance/1.3/SectionsStyle.qml	2016-02-29 16:02:50 +0000
+++ src/Ubuntu/Components/Themes/Ambiance/1.3/SectionsStyle.qml	2016-03-02 16:09:27 +0000
@@ -236,6 +236,7 @@
             }
 
             SmoothedAnimation {
+                objectName: "sections_scroll_animation"
                 id: contentXAnim
                 target: sectionsListView
                 property: "contentX"
@@ -251,48 +252,47 @@
 
         property real iconsDisabledOpacity: 0.3
 
-        property bool hoveringLeft: false
-        property bool hoveringRight: false
-
-        function checkHovering(mouse) {
-            if (mouse.x < listViewContainer.listViewMargins) {
-                if (!hoveringLeft) hoveringLeft = true;
-            } else if (mouse.x > width - listViewContainer.listViewMargins) {
-                if (!hoveringRight) hoveringRight = true;
-            } else {
-                hoveringLeft = false;
-                hoveringRight = false;
-            }
-        }
-
         anchors.fill: parent
         hoverEnabled: true
 
-        onPositionChanged: checkHovering(mouse)
-        onExited: {
-            hoveringLeft = false;
-            hoveringRight = false;
-        }
+        property bool pressedLeft: false
+        property bool pressedRight: false
         onPressed: {
-            if (!hoveringLeft && !hoveringRight) {
-                mouse.accepted = false;
-            }
+            pressedLeft = leftIcon.contains(mouse);
+            pressedRight = rightIcon.contains(mouse);
+            mouse.accepted = pressedLeft || pressedRight;
         }
         onClicked: {
             // positionViewAtIndex() does not provide animation
+            var scrollDirection = 0;
+            if (pressedLeft && leftIcon.contains(mouse)) {
+                scrollDirection = -1;
+            } else if (pressedRight && rightIcon.contains(mouse)) {
+                scrollDirection = 1;
+            } else {
+                // User pressed on the left or right icon, and then released inside of the
+                // MouseArea but outside of the icon that was pressed.
+                return;
+            }
             if (contentXAnim.running) contentXAnim.stop();
-            var newContentX = sectionsListView.contentX + (sectionsListView.width * (hoveringLeft ? -1 : 1));
+            var newContentX = sectionsListView.contentX + (sectionsListView.width * scrollDirection);
             contentXAnim.from = sectionsListView.contentX;
             // make sure we don't overshoot bounds
             contentXAnim.to = MathUtils.clamp(
                         newContentX,
-                        sectionsListView.originX,
+                        0.0, // estimation of originX is some times wrong when scrolling towards the beginning
                         sectionsListView.originX + sectionsListView.contentWidth - sectionsListView.width);
             contentXAnim.start();
         }
 
         Icon {
-            id: leftHoveringIcon
+            id: leftIcon
+            objectName: "left_scroll_icon"
+            // return whether the pressed event was done inside the area of the icon
+            function contains(mouse) {
+                return (mouse.x < listViewContainer.listViewMargins &&
+                        !sectionsListView.atXBeginning);
+            }
             anchors {
                 left: parent.left
                 leftMargin: (listViewContainer.listViewMargins - width) / 2
@@ -315,7 +315,13 @@
         }
 
         Icon {
-            id: rightHoveringIcon
+            id: rightIcon
+            objectName: "right_scroll_icon"
+            // return whether the pressed event was done inside the area of the icon
+            function contains(mouse) {
+                return (mouse.x > (hoveringArea.width - listViewContainer.listViewMargins) &&
+                        !sectionsListView.atXEnd);
+            }
             anchors {
                 right: parent.right
                 rightMargin: (listViewContainer.listViewMargins - width) / 2
@@ -387,8 +393,8 @@
             when: hoveringArea.containsMouse
             PropertyChanges { target: mask; visible: true }
             PropertyChanges { target: listViewContainer; opacity: 0.0 }
-            PropertyChanges { target: leftHoveringIcon; visible: true; }
-            PropertyChanges { target: rightHoveringIcon; visible: true; }
+            PropertyChanges { target: leftIcon; visible: true; }
+            PropertyChanges { target: rightIcon; visible: true; }
         }
     ]
 }

=== modified file 'tests/unit_x11/tst_components/tst_sections.qml'
--- tests/unit_x11/tst_components/tst_sections.qml	2016-02-19 11:18:51 +0000
+++ tests/unit_x11/tst_components/tst_sections.qml	2016-03-02 16:09:27 +0000
@@ -15,6 +15,7 @@
  */
 
 import QtQuick 2.4
+import QtTest 1.0
 import Ubuntu.Test 1.0
 import Ubuntu.Components 1.3
 
@@ -88,6 +89,7 @@
         Sections {
             id: enabledSections
             actions: root.actionList
+            width: parent.width // set width > implicitWidth to test for bug 1551356 below.
         }
         Sections {
             id: disabledSections
@@ -158,10 +160,16 @@
             enabledStringSections.selectedIndex = 0;
             disabledStringSections.selectedIndex = 0;
             label.text = "No action triggered."
+            wait_for_animation(enabledSections);
         }
 
         function wait_for_animation(sections) {
-            // TODO when animations are added
+            var listView = findChild(sections, "sections_listview");
+            if (!listView.moving) {
+                // wait for potential animation to start
+                wait(100);
+            }
+            tryCompare(listView, "moving", false, 2000);
         }
 
         function get_number_of_section_buttons(sections) {
@@ -364,6 +372,53 @@
             wait_for_animation(selectedIndexSections);
         }
 
+        SignalSpy {
+            id: contentXChangedSpy
+            signalName: "contentXChanged"
+        }
+        function test_click_disabled_scroll_button_bug1551356() {
+            var listView = findChild(enabledSections, "sections_listview");
+            var icon = findChild(enabledSections, "left_scroll_icon");
+            contentXChangedSpy.target = listView;
+            contentXChangedSpy.clear();
+            compare(listView.contentX, 0.0, "listView is not at the leftmost position initially.");
+            mouseClick(icon, icon.width/2, icon.height/2);
+            wait(200); // give the listview ample time to scroll
+            compare(contentXChangedSpy.count, 0,
+                    "listView moved when clicking disabled scroll button.");
+            contentXChangedSpy.clear();
+            contentXChangedSpy.target = null;
+        }
+
+        SignalSpy {
+            id: animationStartedSpy
+            signalName: "started"
+        }
+        function test_press_release_on_different_icons() {
+            var leftIcon = findChild(scrollingSections, "left_scroll_icon");
+            var rightIcon = findChild(scrollingSections, "right_scroll_icon");
+            var animation = findInvisibleChild(scrollingSections, "sections_scroll_animation");
+            animationStartedSpy.target = animation;
+            compare(leftIcon.width, rightIcon.width, "Scroll icons are not the same width.");
+            compare(leftIcon.height, rightIcon.height, "Scroll icons are not the same height.");
+            var w = leftIcon.width / 2;
+            var h = leftIcon.height / 2;
+            mouseMove(leftIcon, w, h);
+            mousePress(leftIcon, w, h);
+            mouseMove(rightIcon, w, h);
+            mouseRelease(rightIcon, w, h);
+            wait(200);
+            compare(animationStartedSpy.count, 0,
+                    "Clicked signal came after pressing left icon and releasing on right icon.");
+            mousePress(rightIcon, w, h);
+            mouseMove(leftIcon, w, h);
+            mouseRelease(leftIcon, w, h);
+            wait(200);
+            compare(animationStartedSpy.count, 0,
+                    "Clicked signal came after pressing right icon and releasing on left icon.");
+            animationStartedSpy.target = null;
+        }
+
         function test_keyboard_navigation_data() {
             return [
                         { tag: "actions",