Merge lp:~cimi/unity8/fix-1195349 into lp:unity8

Proposed by Andrea Cimitan
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 467
Merged at revision: 474
Proposed branch: lp:~cimi/unity8/fix-1195349
Merge into: lp:unity8
Diff against target: 47 lines (+5/-4)
1 file modified
Components/Carousel.qml (+5/-4)
To merge this branch: bzr merge lp:~cimi/unity8/fix-1195349
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+191460@code.launchpad.net

Commit message

Fix 1195349 by counting drawbuffer on the newContentX logic of the carousel

Description of the change

When we changed carousel from repeater to listview, we added drawbuffer. this breaks the logic of newContentX, which was considered disabled and set to -1. The correct disabled value now has to take into account the drawbuffer.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:467
http://jenkins.qa.ubuntu.com/job/unity8-ci/1438/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/5073
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/3013
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-saucy/2306
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-amd64-ci/461
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1438
        deb: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1438/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-i386-ci/1437
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/1241
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/948
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/948/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3015
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3015/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2520
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2564
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/49
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/48

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1438/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Can this be autotested?

review: Needs Information
Revision history for this message
Andrea Cimitan (cimi) wrote :

> Can this be autotested?

Don't think so. We currently cover the maths for the carousel, but the real UI is not really easily testable. I think it's better to do manual testing and have the human feedback for it. If something breaks you can spot it immediately, while a computer might test only few cases but not the whole dynamic picture (that the carousel is about)...

Revision history for this message
Albert Astals Cid (aacid) wrote :

Works

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Components/Carousel.qml'
2--- Components/Carousel.qml 2013-07-17 16:27:23 +0000
3+++ Components/Carousel.qml 2013-10-16 16:40:03 +0000
4@@ -72,7 +72,7 @@
5 id: listView
6
7 property real minimumTileWidth: 0
8- property real newContentX: -1
9+ property real newContentX: disabledNewContentX
10 property real pathItemCount: referenceWidth / referenceTileWidth
11 property real tileAspectRatio: 1
12
13@@ -100,6 +100,7 @@
14 readonly property real kMiddleIndex: (realWidth / 2) / tileWidth - 0.5
15 readonly property real kXBeginningEnd: 1 / tileWidth + kMiddleIndex / gapToMiddlePhase
16 readonly property real maximumItemTranslation: (listView.tileWidth * 3) / listView.scaleFactor
17+ readonly property real disabledNewContentX: -carousel.drawBuffer - 1
18 readonly property real realContentWidth: contentWidth - 2 * carousel.drawBuffer
19 readonly property real realContentX: contentX + carousel.drawBuffer
20 readonly property real realPathItemCount: Math.min(realWidth / tileWidth, pathItemCount)
21@@ -192,7 +193,7 @@
22 onMovementStarted: {
23 stepAnimation.stop()
24 newContentXAnimation.stop()
25- newContentX = -1
26+ newContentX = disabledNewContentX
27 }
28 onMovementEnded: {
29 if (realContentX > 0 && realContentX < realContentWidth - realWidth)
30@@ -228,7 +229,7 @@
31 easing.type: Easing.InOutQuad
32 }
33 ScriptAction {
34- script: listView.newContentX = -1
35+ script: listView.newContentX = listView.disabledNewContentX
36 }
37 }
38
39@@ -253,7 +254,7 @@
40 readonly property bool explicitScale: (!listView.moving ||
41 listView.realContentX <= 0 ||
42 listView.realContentX >= listView.realContentWidth - listView.realWidth) &&
43- listView.newContentX < 0 &&
44+ listView.newContentX === listView.disabledNewContentX &&
45 index === listView.selectedIndex
46 readonly property real cachedTiles: listView.realPathItemCount + carousel.drawBuffer / listView.tileWidth
47 readonly property real distance: listView.continuousIndex - index

Subscribers

People subscribed via source and target branches