Unity 8

Merge lp:~nick-dedekind/unity8/indicator.multi-icon into lp:unity8

indicator.multi-icon
Merge into trunk

Proposed by Nick Dedekind on 2013-08-23

Status:	Merged
Approved by:	Michael Zanetti on 2013-09-03
Approved revision:	217
Merged at revision:	284
Proposed branch:	lp:~nick-dedekind/unity8/indicator.multi-icon
Merge into:	lp:unity8
Diff against target:	263 lines (+100/-40) 5 files modified Panel/Indicators/DefaultIndicatorWidget.qml (+1/-1) Panel/Indicators/DefaultIndicatorWidget2.qml (+43/-16) Panel/Indicators/NetworkIndicatorWidget.qml (+1/-1) plugins/Unity/Indicators/rootactionstate.cpp (+46/-16) plugins/Unity/Indicators/rootactionstate.h (+9/-6)
To merge this branch:	bzr merge lp:~nick-dedekind/unity8/indicator.multi-icon
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2013-09-04
Daniel d'Andrada (community)			Approve on 2013-09-03
Albert Astals Cid (community)		2013-08-23	Needs Information on 2013-08-27
Review via email: mp+181862@code.launchpad.net

Commit message

Multiple icon/label support for indicators.

Description of the change

Multiple icon/label support for indicators.

Format:
pre-label
icon(s)
label

lp:~nick-dedekind/unity8/indicator.multi-icon updated on 2013-08-23

214. By Nick Dedekind on 2013-08-23: respaced icons

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-23:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-23:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-26:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-27:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-27:

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

review: Approve (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-08-27:

0.7 grid units seems a weird number. In theory we're told to never use decimal grid units, but i don't mind using 0.5 as you do somewhere else since
\li Most laptops
\li 1 gu = 8 px

\li Retina laptops
\li 1 gu = 16 px

\li Phone with 4 inch screen at HD resolution (around 720x1,280 pixels)
\li 1 gu = 18 px

\li Tablet with 10 inch screen at HD resolution (around 720x1,280 pixels)
\li 1 gu = 10 px

So 0.5 gu will 99.99% still be an exact number of pixels, but 0.7gu mean 5.6, 10.2, 12.6 and 7 pixels respectively, which is a bit weird to declare that we want something to be 5.6 pixels width

So, the question is: Why 0.7?

review: Needs Information

lp:~nick-dedekind/unity8/indicator.multi-icon updated on 2013-08-27

215. By Nick Dedekind on 2013-08-27: pixel boundary respacing.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-27:

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

review: Approve (continuous-integration)

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2013-08-27:

> 0.7 grid units seems a weird number. In theory we're told to never use decimal
> grid units, but i don't mind using 0.5 as you do somewhere else since
> \li Most laptops
> \li 1 gu = 8 px
>
> \li Retina laptops
> \li 1 gu = 16 px
>
> \li Phone with 4 inch screen at HD resolution (around 720x1,280 pixels)
> \li 1 gu = 18 px
>
> \li Tablet with 10 inch screen at HD resolution (around 720x1,280
> pixels)
> \li 1 gu = 10 px
>
> So 0.5 gu will 99.99% still be an exact number of pixels, but 0.7gu mean 5.6,
> 10.2, 12.6 and 7 pixels respectively, which is a bit weird to declare that we
> want something to be 5.6 pixels width
>
> So, the question is: Why 0.7?

Respaced to 0.5 gu.

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2013-09-03:

From DefaultIndicatorWidget2.qml:

"""
    onActionStateChanged: {
        if (actionState == undefined) {
            label = "";
            iconSource = "";
            enabled = false;
            return;
        }
"""

Forgot to update the contents of this "if".

review: Needs Fixing

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2013-09-03:

In plugins/Unity/Indicators/rootactionstate.cpp

@@ -198,16 +205,38 @@ QVariant RootActionState::toQVariant(GVa
         while (g_variant_iter_loop (&iter, "{sv}", &key, &vvalue))
         {
             QString str = QString::fromUtf8(key);
- if (str == "icon") {
+ if (str == "icon" && !qmap.contains("icons")) {
+ QStringList icons;
+
                 // FIXME - should be sending a url.
- GIcon *icon = g_icon_deserialize (vvalue);
- if (icon) {
- qmap.insert(str, iconUri(icon));
- g_object_unref (icon);
+ GIcon *gicon = g_icon_deserialize (vvalue);
+ if (gicon) {
+ icons << iconUri(gicon);
                 }
- else {
- qmap.insert(str, QVariant(""));
+ qmap.insert("icons", icons);

I think you should keep the "g_object_unref(icon);" call on the icon created by "g_icon_deserialize(vvalue)".

review: Needs Fixing

lp:~nick-dedekind/unity8/indicator.multi-icon updated on 2013-09-03

216. By Nick Dedekind on 2013-09-03: fixed up some old variables
217. By Nick Dedekind on 2013-09-03: added unref for icon

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2013-09-03:

> In plugins/Unity/Indicators/rootactionstate.cpp
>
> @@ -198,16 +205,38 @@ QVariant RootActionState::toQVariant(GVa
> while (g_variant_iter_loop (&iter, "{sv}", &key, &vvalue))
> {
> QString str = QString::fromUtf8(key);
> - if (str == "icon") {
> + if (str == "icon" && !qmap.contains("icons")) {
> + QStringList icons;
> +
> // FIXME - should be sending a url.
> - GIcon *icon = g_icon_deserialize (vvalue);
> - if (icon) {
> - qmap.insert(str, iconUri(icon));
> - g_object_unref (icon);
> + GIcon *gicon = g_icon_deserialize (vvalue);
> + if (gicon) {
> + icons << iconUri(gicon);
> }
> - else {
> - qmap.insert(str, QVariant(""));
> + qmap.insert("icons", icons);
>
> I think you should keep the "g_object_unref(icon);" call on the icon created
> by "g_icon_deserialize(vvalue)".

Thanks. Fixed.

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2013-09-03:

> From DefaultIndicatorWidget2.qml:
>
> """
> onActionStateChanged: {
> if (actionState == undefined) {
> label = "";
> iconSource = "";
> enabled = false;
> return;
> }
> """
>
> Forgot to update the contents of this "if".

Fixed.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-03:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-03:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2013-09-03:

Looks good.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) on 2013-09-04:

review: Approve (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:

Mohammad Rakibul Hasan

Nick Dedekind

Unity Team

 === modified file 'Panel/Indicators/DefaultIndicatorWidget.qml'
 --- Panel/Indicators/DefaultIndicatorWidget.qml	2013-08-14 09:07:45 +0000
 +++ Panel/Indicators/DefaultIndicatorWidget.qml	2013-09-03 17:19:05 +0000
@@ -24,7 +24,7 @@
  Indicators.IndicatorWidget {
      id: indicatorWidget
--    width: itemRow.width + units.gu(1)
++    width: itemRow.width + units.gu(0.5)
      property alias label: itemLabel.text
      property alias iconSource: itemImage.source
 === modified file 'Panel/Indicators/DefaultIndicatorWidget2.qml'
 --- Panel/Indicators/DefaultIndicatorWidget2.qml	2013-08-14 09:07:45 +0000
 +++ Panel/Indicators/DefaultIndicatorWidget2.qml	2013-09-03 17:19:05 +0000
@@ -24,10 +24,11 @@
  Indicators.IndicatorWidget {
      id: indicatorWidget
--    width: itemRow.width + units.gu(1)
++    width: itemRow.width + units.gu(0.5)
--    property alias label: itemLabel.text
--    property alias iconSource: itemImage.source
++    property alias leftLabel: itemLeftLabel.text
++    property alias rightLabel: itemRightLabel.text
++    property var icons: undefined
      Row {
          id: itemRow
@@ -39,19 +40,43 @@
+         }
          spacing: units.gu(0.5)
--        // FIXME : Should us Ubuntu.Icon . results in low res images
--        Image {
--            id: itemImage
--            objectName: "itemImage"
--            visible: source != ""
--            height: indicatorWidget.iconSize
--            width: indicatorWidget.iconSize
++        Label {
++            id: itemLeftLabel
++            objectName: "leftLabel"
++            color: Theme.palette.selected.backgroundText
++            opacity: 0.8
++            font.family: "Ubuntu"
++            fontSize: "medium"
              anchors.verticalCenter: parent.verticalCenter
++            visible: text != ""
++        }
++
++        Row {
++            width: childrenRect.width
++            anchors {
++                top: parent.top
++                bottom: parent.bottom
++            }
++            spacing: units.gu(0.5)
++
++            Repeater {
++                model: indicatorWidget.icons
++
++                Image {
++                    id: itemImage
++                    objectName: "itemImage"
++                    visible: source != ""
++                    source: modelData
++                    height: indicatorWidget.iconSize
++                    width: indicatorWidget.iconSize
++                    anchors.verticalCenter: parent.verticalCenter
++                }
++            }
+         }
          Label {
--            id: itemLabel
--            objectName: "itemLabel"
++            id: itemRightLabel
++            objectName: "rightLabel"
              color: Theme.palette.selected.backgroundText
              opacity: 0.8
              font.family: "Ubuntu"
@@ -63,14 +88,16 @@
      onActionStateChanged: {
          if (actionState == undefined) {
--            label = "";
--            iconSource = "";
++            leftLabel = "";
++            rightLabel = "";
++            icons = undefined;
              enabled = false;
              return;
+         }
--        label = actionState.label;
--        iconSource = actionState.icon;
++        leftLabel = actionState.leftLabel ? actionState.leftLabel : "";
++        rightLabel = actionState.rightLabel ? actionState.rightLabel : "";
++        icons = actionState.icons;
          enabled = actionState.visible;
+     }
+ }
 === modified file 'Panel/Indicators/NetworkIndicatorWidget.qml'
 --- Panel/Indicators/NetworkIndicatorWidget.qml	2013-08-14 09:07:45 +0000
 +++ Panel/Indicators/NetworkIndicatorWidget.qml	2013-09-03 17:19:05 +0000
@@ -24,7 +24,7 @@
  Indicators.IndicatorWidget {
      id: indicatorWidget
--    width: networkIcon.width + units.gu(1)
++    width: networkIcon.width + units.gu(0.5)
      property int signalStrength: 0
      property int connectionState: Indicators.NetworkConnection.Initial
 === modified file 'plugins/Unity/Indicators/rootactionstate.cpp'
 --- plugins/Unity/Indicators/rootactionstate.cpp	2013-08-14 09:07:45 +0000
 +++ plugins/Unity/Indicators/rootactionstate.cpp	2013-09-03 17:19:05 +0000
@@ -108,25 +108,32 @@
      return m_menu && m_menu->rowCount() > 0;
+ }
--QString RootActionState::label() const
++QString RootActionState::leftLabel() const
++{
++    if (!isValid()) return QString();
++
++    return m_cachedState.value("pre-label", QVariant::fromValue(QString())).toString();
++}
++
++QString RootActionState::rightLabel() const
+ {
      if (!isValid()) return QString();
      return m_cachedState.value("label", QVariant::fromValue(QString())).toString();
+ }
--QString RootActionState::icon() const
++QStringList RootActionState::icons() const
+ {
--    if (!isValid()) return QString();
++    if (!isValid()) return QStringList();
--    return m_cachedState.value("icon", QVariant::fromValue(QString())).toString();
++    return m_cachedState.value("icons", QVariant::fromValue(QStringList())).toStringList();
+ }
  QString RootActionState::accessibleName() const
+ {
      if (!isValid()) return QString();
--    return m_cachedState.value("accessible-name", QVariant::fromValue(QString())).toString();
++    return m_cachedState.value("accessible-desc", QVariant::fromValue(QString())).toString();
+ }
  bool RootActionState::isVisible() const
@@ -198,16 +205,39 @@
          while (g_variant_iter_loop (&iter, "{sv}", &key, &vvalue))
+         {
              QString str = QString::fromUtf8(key);
--            if (str == "icon") {
++            if (str == "icon" && !qmap.contains("icons")) {
++                QStringList icons;
++
                  // FIXME - should be sending a url.
--                GIcon *icon = g_icon_deserialize (vvalue);
--                if (icon) {
--                    qmap.insert(str, iconUri(icon));
--                    g_object_unref (icon);
--                }
--                else {
--                    qmap.insert(str, QVariant(""));
--                }
++                GIcon *gicon = g_icon_deserialize (vvalue);
++                if (gicon) {
++                    icons << iconUri(gicon);
++                    g_object_unref (gicon);
++                }
++                qmap.insert("icons", icons);
++
++            } else if (str == "icons") {
++
++                QStringList icons;
++
++                if (g_variant_type_is_array(g_variant_get_type(vvalue))) {
++
++
++                    for (int i = 0, iMax = g_variant_n_children(vvalue); i < iMax; i++) {
++                        GVariant *child = g_variant_get_child_value(vvalue, i);
++
++                        // FIXME - should be sending a url.
++                        GIcon *gicon = g_icon_deserialize (child);
++                        if (gicon) {
++                            icons << iconUri(gicon);
++                            g_object_unref (gicon);
++                        }
++                        g_variant_unref(child);
++                    }
++                }
++                // will overwrite icon.
++                qmap.insert("icons", icons);
++
              } else {
                  qmap.insert(str, ActionStateParser::toQVariant(vvalue));
+             }
@@ -230,12 +260,12 @@
                                         &visible);
          qmap["label"] = label ? QString::fromUtf8(label) : "";
--        qmap["accessible-name"] = accessible_name ? QString::fromUtf8(accessible_name) : "";
++        qmap["accessible-desc"] = accessible_name ? QString::fromUtf8(accessible_name) : "";
          qmap["visible"] = visible;
          gicon = g_icon_new_for_string (icon, NULL);
          if (gicon) {
--            qmap["icon"] = iconUri(gicon);
++            qmap["icons"] = QStringList() << iconUri(gicon);
              g_object_unref (gicon);
+         }
 === modified file 'plugins/Unity/Indicators/rootactionstate.h'
 --- plugins/Unity/Indicators/rootactionstate.h	2013-08-14 09:07:45 +0000
 +++ plugins/Unity/Indicators/rootactionstate.h	2013-09-03 17:19:05 +0000
@@ -32,8 +32,9 @@
      Q_PROPERTY(UnityMenuModel* menu READ menu WRITE setMenu NOTIFY menuChanged)
      Q_PROPERTY(bool valid READ isValid NOTIFY validChanged)
--    Q_PROPERTY(QString label READ label NOTIFY labelChanged)
--    Q_PROPERTY(QString icon READ icon NOTIFY iconChanged)
++    Q_PROPERTY(QString leftLabel READ leftLabel NOTIFY leftLabelChanged)
++    Q_PROPERTY(QString rightLabel READ rightLabel NOTIFY rightLabelChanged)
++    Q_PROPERTY(QStringList icons READ icons NOTIFY iconsChanged)
      Q_PROPERTY(QString accessibleName READ accessibleName NOTIFY accessibleNameChanged)
      Q_PROPERTY(bool visible READ isVisible NOTIFY visibleChanged)
  public:
@@ -47,8 +48,9 @@
      void setIndex(int index);
      bool isValid() const;
--    QString label() const;
--    QString icon() const;
++    QString leftLabel() const;
++    QString rightLabel() const;
++    QStringList icons() const;
      QString accessibleName() const;
      bool isVisible() const;
@@ -62,8 +64,9 @@
      void indexChanged();
      void validChanged();
--    void labelChanged();
--    void iconChanged();
++    void leftLabelChanged();
++    void rightLabelChanged();
++    void iconsChanged();
      void accessibleNameChanged();
      void visibleChanged();