Merge lp:~saviq/unity8/fix-empty-attributes into lp:unity8

[Michał Sawicz]

Don't ignore empty attributes in CardAttributes.qml and improve its encapsulation

 * Are there any related MPs required for this MP to build/function as expected? Please list.
N
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Y
 * Did you make sure that your branch does not contain spurious tags?
Y
 * 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

[lp:~saviq/unity8/fix-empty-attributes]

1164. By Michał Sawicz on 2014-08-16

Merge lp:~saviq/unity8/header-customizations

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1163
http://jenkins.qa.ubuntu.com/job/unity8-ci/3946/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3636
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/947
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1040
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1040
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1040/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1040
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3525
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4883
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4883/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11599

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3946/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1164
http://jenkins.qa.ubuntu.com/job/unity8-ci/3965/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3655/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/966
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1059
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1059
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1059/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1059
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3541/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4902
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4902/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11615

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3965/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1164
http://jenkins.qa.ubuntu.com/job/unity8-ci/3979/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3673
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/980
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1073
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1073
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1073/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1073
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3560
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4920
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4920/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11635

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3979/rebuild

review: Needs Fixing (continuous-integration)

[lp:~saviq/unity8/fix-empty-attributes]

1165. By Michał Sawicz on 2014-08-17

Merge lp:~saviq/unity8/header-customizations

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1165
http://jenkins.qa.ubuntu.com/job/unity8-ci/3992/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3687
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/993
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1086
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1086
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1086/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1086
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3571
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4934
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4934/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11647

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3992/rebuild

review: Needs Fixing (continuous-integration)

[lp:~saviq/unity8/fix-empty-attributes]

1166. By Michał Sawicz on 2014-08-18

Fix cardcreator test.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1166
http://jenkins.qa.ubuntu.com/job/unity8-ci/3993/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3694
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/994
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1087
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1087
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/1087/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1087
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4941
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4941/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11663

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3993/rebuild

review: Approve (continuous-integration)

[Andrea Cimitan (cimi)]

if you test cardAttributes with model: testData[4] you see the text omitted, something is not right there...

review: Needs Fixing

[Michał Sawicz (saviq)]

Right, that's a caveat: the first row determines column widths for the rest of the rows. So if you have an empty attribute in the first row, any below that one in the same column will not render properly.

It's a limitation resulting from a desire for this all to be somewhat dynamic. We should probably revisit the rules for laying them out soon. We might, for example, put them in a Column of RowLayouts instead of a GridLayout, knowing that this will cause uneven horizontal centering of the middle column. But maybe that'd be a better case than what we have now. We'll need to weigh this out with design.

[Andrea Cimitan (cimi)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
y

review: Approve

=== modified file 'plugins/Dash/CardAttributes.qml'
--- plugins/Dash/CardAttributes.qml	2014-08-18 08:22:56 +0000
+++ plugins/Dash/CardAttributes.qml	2014-08-18 08:22:57 +0000
@@ -26,8 +26,10 @@
         right: parent.right;
     }
     columns: 2 + repeater.count % 2
+    rowSpacing: units.gu(.5)
     property alias model: repeater.model
-    property var color
+    property color color: Theme.palette.normal.baseText
+    property real fontScale: 1.0
 
     Repeater {
         id: repeater
@@ -42,18 +44,19 @@
             Layout.columnSpan: index == repeater.count - 1 && grid.columns == 3 && column == 1 ? 2 : 1
             Layout.maximumWidth: Math.max(icon.width, label.x + label.implicitWidth)
             Layout.fillWidth: true
+            height: units.gu(2)
             StatusIcon {
                 id: icon
                 height: units.gu(2)
                 sets: ["actions", "status", "apps"]
-                source: "icon" in modelData ? modelData["icon"] : ""
+                source: "icon" in modelData && modelData["icon"] || ""
                 color: grid.color
             }
             Label {
                 id: label
                 width: parent.width - x
                 anchors.verticalCenter: parent.verticalCenter
-                text: "value" in modelData ? modelData["value"] : "";
+                text: "value" in modelData && modelData["value"] || "";
                 elide: Text.ElideRight
                 maximumLineCount: 1
                 font.weight: "style" in modelData && modelData["style"] == "highlighted" ? Font.DemiBold : Font.Light

=== modified file 'plugins/Dash/CardCreator.js'
--- plugins/Dash/CardCreator.js	2014-08-18 08:22:56 +0000
+++ plugins/Dash/CardCreator.js	2014-08-18 08:22:57 +0000
@@ -274,6 +274,7 @@
                             objectName: "attributesRow"; \n\
                             anchors { %1 } \n\
                             color: %2; \n\
+                            fontScale: root.fontScale; \n\
                             model: cardData && cardData["attributes"]; \n\
                           }\n';
 

=== modified file 'tests/plugins/Dash/cardcreator/7.res'
--- tests/plugins/Dash/cardcreator/7.res	2014-08-18 08:22:56 +0000
+++ tests/plugins/Dash/cardcreator/7.res	2014-08-18 08:22:57 +0000
@@ -128,6 +128,7 @@
                             top: subtitleLabel.bottom; 
                             } 
                             color: backgroundLoader.active && backgroundLoader.item && root.scopeStyle ? root.scopeStyle.getTextColor(backgroundLoader.item.luminance) : (backgroundLoader.item.luminance > 0.7 ? Theme.palette.normal.baseText : "white");
+                            fontScale: root.fontScale; 
                             model: cardData && cardData["attributes"]; 
                           }
  

=== modified file 'tests/plugins/Dash/tst_CardAttributes.qml'
--- tests/plugins/Dash/tst_CardAttributes.qml	2014-07-25 08:46:29 +0000
+++ tests/plugins/Dash/tst_CardAttributes.qml	2014-08-18 08:22:57 +0000
@@ -26,7 +26,8 @@
         [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"}],
         [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"},{"value":"text3"}],
         [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"},{"value":"text3"},{"value":"text4"}],
-        [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"},{"value":"text3","style":"highlighted"},{"value":"text4","icon":"image://theme/close","style":"highlighted"},{"value":"text5"}]
+        [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"},{"value":"text3","style":"highlighted"},{"value":"text4","icon":"image://theme/close","style":"highlighted"},{"value":"text5"}],
+        [{"value":"text1","icon":"image://theme/ok"},{},{},{},{"value":"text5", "icon": "image://theme/search"}],
     ]
 
     CardAttributes {
@@ -49,6 +50,8 @@
         function test_columns(data) {
             cardAttributes.model = data;
             compare(cardAttributes.columns, 2 + data.length % 2);
+            var rows = Math.ceil(data.length / 3);
+            tryCompare(cardAttributes, "height", rows * units.gu(2) + (rows - 1) * cardAttributes.rowSpacing);
         }
     }
 }