Unity 8

Merge lp:~aacid/unity8/fix-1599238 into lp:unity8

fix-1599238
Merge into trunk

Proposed by Albert Astals Cid on 2016-07-06

Status:

Merged

Approved by:

Andrea Cimitan on 2016-08-02

Approved revision:

2529

Merged at revision:

2578

Proposed branch:

lp:~aacid/unity8/fix-1599238

Merge into:

lp:unity8

Diff against target:

53 lines (+25/-1)

2 files modified

qml/Dash/CardCarousel.qml (+1/-1)
tests/qmltests/Dash/tst_CardCarousel.qml (+24/-0)

To merge this branch:

bzr merge lp:~aacid/unity8/fix-1599238

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-08-11
Andrea Cimitan (community)		2016-07-06	Approve on 2016-08-02
Review via email: mp+299312@code.launchpad.net

Commit Message

Take into account carousel selectedItemScaleFactor when setting card fixedArtShapeSize

Description of the Change

* Are there any related MPs required for this MP to build/function as expected?
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-07-06:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1677/rebuild

review: Needs Fixing (continuous-integration)

Andrea Cimitan (cimi) wrote on 2016-07-07:

Tested, works fine. You think is worth adding a small test for it in tst_CardCarousel or we will drop it soon?

review: Needs Information

lp:~aacid/unity8/fix-1599238 updated on 2016-07-11

2529. By Albert Astals Cid on 2016-07-11: Add test

Albert Astals Cid (aacid) wrote on 2016-07-11:

> Tested, works fine. You think is worth adding a small test for it in
> tst_CardCarousel or we will drop it soon?

Added test

Andrea Cimitan (cimi) wrote on 2016-07-11:

just waiting CI

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-07-13:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1711/rebuild

review: Needs Fixing (continuous-integration)

Andrea Cimitan (cimi) on 2016-08-02:

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-08-11:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1938/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

 === modified file 'qml/Dash/CardCarousel.qml'
 --- qml/Dash/CardCarousel.qml	2016-05-20 15:56:34 +0000
 +++ qml/Dash/CardCarousel.qml	2016-07-11 09:39:09 +0000
@@ -62,7 +62,7 @@
              asynchronous: true
              onLoaded: {
                  item.fixedHeaderHeight = Qt.binding(function() { return carousel.headerHeight; });
--                item.fixedArtShapeSize = Qt.binding(function() { return cardTool.artShapeSize; });
++                item.fixedArtShapeSize = Qt.binding(function() { return Qt.size(cardTool.artShapeSize.width / carousel.selectedItemScaleFactor, cardTool.artShapeSize.height / carousel.selectedItemScaleFactor); });
                  item.height = Qt.binding(function() { return cardTool.cardHeight; });
                  item.cardData = Qt.binding(function() { return model; });
                  item.fontScale = Qt.binding(function() { return carousel.fontScale; });
 === modified file 'tests/qmltests/Dash/tst_CardCarousel.qml'
 --- tests/qmltests/Dash/tst_CardCarousel.qml	2015-08-28 12:39:43 +0000
 +++ tests/qmltests/Dash/tst_CardCarousel.qml	2016-07-11 09:39:09 +0000
@@ -70,6 +70,15 @@
          when: windowShown
++        function findScale(item) {
++            var scale = 1;
++            while (item) {
++                scale = scale * item.scale;
++                item = item.parent;
++            }
++            return scale;
++        }
++
          function test_non_overlaid_header_layout() {
              // When the header is non overlaid
              // we grow down to accomodate the header but the y of the list is the same
@@ -83,5 +92,20 @@
              compare(listView.y, yPos);
              compare(cardCarousel.height, height + cTool.headerHeight);
+         }
++
++        function test_item_fits_view_data() {
++            return [
++                {tag: "overlaid", template: overlaidTemplate},
++                {tag: "nonOverlaid", template: nonOverlaidTemplate},
++            ];
++        }
++
++        function test_item_fits_view(data) {
++            cTool.template = data.template;
++            var delegate0 = findChild(cardCarousel, "carouselDelegate0");
++            tryCompareFunction(function() { return delegate0.item != null; }, true);
++            var scale = findScale(delegate0.item);
++            verify(delegate0.item.implicitHeight * scale < cardCarousel.height);
++        }
+     }
+ }