Merge lp:~nick-dedekind/unity8/lp1236267 into lp:unity8

[Nick Dedekind]

Round indicator widget icon/label width up to closest gu for alignment.

Round indicator widget icon/label width up to closest gu for alignment.
Preserve icon aspect ratio.

Added test for DefaultIndicatorWidget width rounding.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:466
http://jenkins.qa.ubuntu.com/job/unity8-ci/1421/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/5047
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/2985/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-saucy/2289
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-amd64-ci/444
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1421
        deb: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1421/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-i386-ci/1420
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/1217
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/922
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/922/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/2987
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/2987/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2493/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2537/console

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Do you think it makes sense for guRoundUp to be somewhere more central so it can be eventually reused? Or maybe do it when we need to?

review: Needs Information

[Michał Sawicz (saviq)]

On 17.10.2013 11:24, Albert Astals Cid wrote:
> Do you think it makes sense for guRoundUp to be somewhere more central so it can be eventually reused? Or maybe do it when we need to?

Maybe we should even put it in the UITK for others to use? I.e. put it
on units.guRoundUp()?

[Nick Dedekind (nick-dedekind)]

> On 17.10.2013 11:24, Albert Astals Cid wrote:
> > Do you think it makes sense for guRoundUp to be somewhere more central so it
> can be eventually reused? Or maybe do it when we need to?
>
> Maybe we should even put it in the UITK for others to use? I.e. put it
> on units.guRoundUp()?

I'll put add a request to the toolkit, and up a TODO in here. Don't think it should be held up until we get that in though.

[Nick Dedekind (nick-dedekind)]

Added TODO for https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1242575

[Albert Astals Cid (aacid)]

Ok

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:467
http://jenkins.qa.ubuntu.com/job/unity8-ci/1454/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/5107
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/3081/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-saucy/2325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-amd64-ci/477
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1454/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-i386-ci/1453
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/1271
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/982
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/982/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3083
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3083/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2579/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2630/console
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/189
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/188

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

review: Needs Fixing (continuous-integration)

[Michael Zanetti (mzanetti)]

can we add a test for this?

[Nick Dedekind (nick-dedekind)]

> can we add a test for this?

Yes.
I've put it back into 'Needs review' until tests added.

[Nick Dedekind (nick-dedekind)]

Added DefaultIndicatorWidget width test.
Requires lp:~nick-dedekind/unity8/indicator.submenu-reinit

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:469
http://jenkins.qa.ubuntu.com/job/unity8-ci/1508/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/138/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/132/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/35/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/32
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/32
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/32/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/32/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/129/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/138
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/138/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/132
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/132/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2759/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2809
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/558
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/557

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Looks good

review: Approve

[Michał Sawicz (saviq)]

There's a failed autolanding coming - it's caused by the prerequisite branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/624/
Executed test runs:
    SUCCESS: http://10.97.0.26:8080/job/generic-cleanup-mbs/3061
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/293
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/283
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/81
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/10
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/10/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/277
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/293
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/293/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/283
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/283/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2900
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2951
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/846
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/847

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Pre-requisite got merged, try to land again.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'Panel/Indicators/DefaultIndicatorWidget.qml'
--- Panel/Indicators/DefaultIndicatorWidget.qml	2013-10-02 16:07:39 +0000
+++ Panel/Indicators/DefaultIndicatorWidget.qml	2013-10-28 18:24:51 +0000
@@ -24,7 +24,7 @@
 Indicators.IndicatorWidget {
     id: indicatorWidget
 
-    width: itemRow.width + units.gu(0.5)
+    width: itemRow.width
 
     property alias leftLabel: itemLeftLabel.text
     property alias rightLabel: itemRightLabel.text
@@ -32,16 +32,17 @@
 
     Row {
         id: itemRow
+        width: childrenRect.width
         objectName: "itemRow"
         anchors {
             top: parent.top
             bottom: parent.bottom
             horizontalCenter: parent.horizontalCenter
         }
-        spacing: units.gu(0.5)
 
         Label {
             id: itemLeftLabel
+            width: guRoundUp(implicitWidth)
             objectName: "leftLabel"
             color: Theme.palette.selected.backgroundText
             opacity: 0.8
@@ -57,25 +58,35 @@
                 top: parent.top
                 bottom: parent.bottom
             }
-            spacing: units.gu(0.5)
 
             Repeater {
                 model: indicatorWidget.icons
 
-                Image {
-                    id: itemImage
-                    objectName: "itemImage"
-                    visible: source != ""
-                    source: modelData
+                Item {
+                    width: guRoundUp(itemImage.width)
                     height: indicatorWidget.iconSize
-                    width: indicatorWidget.iconSize
-                    anchors.verticalCenter: parent.verticalCenter
+
+                    Image {
+                        id: itemImage
+                        objectName: "itemImage"
+                        visible: source != ""
+                        source: modelData
+                        height: parent.height
+                        anchors.horizontalCenter: parent.horizontalCenter
+                        fillMode: Image.PreserveAspectFit
+
+                        sourceSize {
+                            width: indicatorWidget.iconSize
+                            height: indicatorWidget.iconSize
+                        }
+                    }
                 }
             }
         }
 
         Label {
             id: itemRightLabel
+            width: guRoundUp(implicitWidth)
             objectName: "rightLabel"
             color: Theme.palette.selected.backgroundText
             opacity: 0.8
@@ -86,6 +97,17 @@
         }
     }
 
+    // TODO: Use toolkit function https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1242575
+    function guRoundUp(width) {
+        if (width == 0) {
+            return 0;
+        }
+        var gu1 = units.gu(1.0);
+        var mod = (width % gu1);
+
+        return mod == 0 ? width : width + (gu1 - mod);
+    }
+
     onRootActionStateChanged: {
         if (rootActionState == undefined) {
             leftLabel = "";

=== modified file 'plugins/Unity/Indicators/qml/IndicatorWidget.qml'
--- plugins/Unity/Indicators/qml/IndicatorWidget.qml	2013-09-30 16:37:55 +0000
+++ plugins/Unity/Indicators/qml/IndicatorWidget.qml	2013-10-28 18:24:51 +0000
@@ -24,5 +24,5 @@
     id: indicatorWidget
 
     enabled: false
-    property int iconSize: height - units.gu(0.5)
+    property int iconSize: height
 }

=== added file 'tests/mocks/Unity/Indicators/IndicatorBase.qml'
--- tests/mocks/Unity/Indicators/IndicatorBase.qml	1970-01-01 00:00:00 +0000
+++ tests/mocks/Unity/Indicators/IndicatorBase.qml	2013-10-28 18:24:51 +0000
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Nick Dedekind <[e-mail]>
+ */
+
+import QtQuick 2.0
+import QMenuModel 0.1
+
+Item {
+    id: indicatorBase
+
+    //const
+    property string busName
+    property string actionsObjectPath
+    property string rootMenuType: "com.canonical.indicator.root"
+    property string deviceMenuObjectPath
+
+    property var menuModel: UnityMenuModel {}
+
+    property RootActionState rootActionState: RootActionState {
+        onUpdated: indicatorBase.rootActionStateChanged()
+    }
+}

=== added file 'tests/mocks/Unity/Indicators/IndicatorWidget.qml'
--- tests/mocks/Unity/Indicators/IndicatorWidget.qml	1970-01-01 00:00:00 +0000
+++ tests/mocks/Unity/Indicators/IndicatorWidget.qml	2013-10-28 18:24:51 +0000
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Nick Dedekind <[e-mail]>
+ */
+
+import QtQuick 2.0
+import Unity.Indicators 0.1 as Indicators
+
+IndicatorBase {
+    id: indicatorWidget
+
+    enabled: false
+    property int iconSize: height
+}

=== modified file 'tests/mocks/Unity/Indicators/RootActionState.qml'
--- tests/mocks/Unity/Indicators/RootActionState.qml	2013-10-11 18:58:51 +0000
+++ tests/mocks/Unity/Indicators/RootActionState.qml	2013-10-28 18:24:51 +0000
@@ -20,14 +20,22 @@
 import QtQuick 2.0
 
 QtObject {
-  property var menu
-  property bool valid: false
-  property string title
-  property string leftLabel
-  property string rightLabel
-  property var icons
-  property string accessibleName
-  property bool visible: true
-
-  signal updated
+    property var menu
+    property bool valid: false
+    property string title
+    property string leftLabel
+    property string rightLabel
+    property var icons
+    property string accessibleName
+    property bool visible: true
+
+    signal updated
+
+    onValidChanged: updated()
+    onTitleChanged: updated()
+    onLeftLabelChanged: updated()
+    onRightLabelChanged: updated()
+    onIconsChanged: updated()
+    onAccessibleNameChanged: updated()
+    onVisibleChanged: updated()
 }

=== modified file 'tests/mocks/Unity/Indicators/qmldir'
--- tests/mocks/Unity/Indicators/qmldir	2013-10-11 18:58:51 +0000
+++ tests/mocks/Unity/Indicators/qmldir	2013-10-28 18:24:51 +0000
@@ -2,6 +2,8 @@
 plugin IndicatorsFakeQml
 
 CachedUnityMenuModel 0.1 CachedUnityMenuModel.qml
+IndicatorBase 0.1 IndicatorBase.qml
+IndicatorWidget 0.1 IndicatorWidget.qml
 IndicatorsModel 0.1 IndicatorsModel.qml
 FakeMenuPage 0.1 FakeMenuPage.qml
 RootActionState 0.1 RootActionState.qml

=== modified file 'tests/qmltests/CMakeLists.txt'
--- tests/qmltests/CMakeLists.txt	2013-10-28 18:24:51 +0000
+++ tests/qmltests/CMakeLists.txt	2013-10-28 18:24:51 +0000
@@ -71,6 +71,8 @@
 add_qml_test(Panel MenuContent IMPORT_PATHS ${CMAKE_BINARY_DIR}/plugins ${qmltest_DEFAULT_IMPORT_PATHS})
 add_qml_test(Panel Panel IMPORT_PATHS ${CMAKE_BINARY_DIR}/plugins ${qmltest_DEFAULT_IMPORT_PATHS})
 add_qml_test(Panel SearchIndicator)
+add_qml_test(Panel/Indicators DefaultIndicatorWidget IMPORT_PATHS ${CMAKE_BINARY_DIR}/plugins ${qmltest_DEFAULT_IMPORT_PATHS}
+                                                  ENVIRONMENT "LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/tests/mocks/QMenuModel")
 add_qml_test(SideStage SideStage IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS}
                                               ${CMAKE_BINARY_DIR}/tests/mocks
                                               ${CMAKE_BINARY_DIR}/plugins)

=== added directory 'tests/qmltests/Panel/Indicators'
=== added file 'tests/qmltests/Panel/Indicators/tst_DefaultIndicatorWidget.qml'
--- tests/qmltests/Panel/Indicators/tst_DefaultIndicatorWidget.qml	1970-01-01 00:00:00 +0000
+++ tests/qmltests/Panel/Indicators/tst_DefaultIndicatorWidget.qml	2013-10-28 18:24:51 +0000
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import QtTest 1.0
+import Unity.Test 0.1 as UT
+import QMenuModel 0.1
+import "../../../../Panel/Indicators"
+
+Item {
+    id: testView
+    width: units.gu(40)
+    height: units.gu(70)
+
+   DefaultIndicatorWidget {
+        id: widget
+
+        anchors {
+            left: parent.left
+            top: parent.top
+        }
+
+        busName: "test"
+        actionsObjectPath: "test"
+        deviceMenuObjectPath: "test"
+
+        rootMenuType: ""
+
+        iconSize: units.gu(3.2)
+        height: units.gu(3)
+
+        rootActionState {
+            icons: [ "image://theme/audio-volume-high", "image://theme/audio-volume-low" ]
+            leftLabel: "left"
+            rightLabel: "right"
+        }
+    }
+
+    UT.UnityTestCase {
+        name: "DefaultIndicatorWidget"
+        when: windowShown
+
+        function init() {
+            widget.rootActionState.icons = [];
+            widget.rootActionState.leftLabel = "";
+            widget.rootActionState.rightLabel = "";
+            wait(50);
+        }
+
+        function test_guRoundedWidth_data() {
+            return [
+                { tag: "empty", icons: [], leftLabel: "", rightLabel: "" },
+                { tag: "1-icon-no-label", icons: [ "image://theme/audio-volume-high" ], leftLabel: "", rightLabel: "" },
+                { tag: "2-icon-no-label", icons: [ "image://theme/audio-volume-high", "image://theme/audio-volume-low" ], leftLabel: "", rightLabel: "" },
+                { tag: "no-icon-l-label", icons: [], leftLabel: "left", rightLabel: "" },
+                { tag: "no-icon-lr-label", icons: [], leftLabel: "left", rightLabel: "right" },
+                { tag: "1-icon-l-label", icons: [ "image://theme/audio-volume-high" ], leftLabel: "left", rightLabel: "" },
+                { tag: "1-icon-lr-label", icons: [ "image://theme/audio-volume-high" ], leftLabel: "left", rightLabel: "right" },
+                { tag: "2-icon-l-label", icons: [ "image://theme/audio-volume-high", "image://theme/audio-volume-low" ], leftLabel: "left", rightLabel: "" },
+                { tag: "2-icon-lr-label", icons: [ "image://theme/audio-volume-high", "image://theme/audio-volume-low" ], leftLabel: "left", rightLabel: "right" }
+            ];
+        }
+
+        function test_guRoundedWidth(data) {
+            widget.rootActionState.icons = data.icons
+            widget.rootActionState.leftLabel = data.leftLabel;
+            widget.rootActionState.rightLabel = data.rightLabel;
+            wait(50);
+
+            compare(widget.width, guRoundUp(widget.width));
+        }
+    }
+
+    // TODO: Use toolkit function https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1242575
+    function guRoundUp(width) {
+        if (width == 0) {
+            return 0;
+        }
+        var gu1 = units.gu(1.0);
+        var mod = (width % gu1);
+
+        return mod == 0 ? width : width + (gu1 - mod);
+    }
+}