Code review comment for lp:~bcsaller/juju-gui/nostatusforoldsubordinates

Reviewers: mp+174842_code.launchpad.net,

Message:
Please take a look.

Description:
Subordinates don't get status bars

Subordinates could render status in the future, but for now we omit
this as the UI relating to mapping sub units to principal service units
is unclear.

https://code.launchpad.net/~bcsaller/juju-gui/nostatusforoldsubordinates/+merge/174842

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/11295043/

Affected files:
   A [revision details]
   M app/views/topology/service.js
   M app/views/utils.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/views/utils.js
=== modified file 'app/views/utils.js'
--- app/views/utils.js 2013-07-11 14:56:15 +0000
+++ app/views/utils.js 2013-07-15 19:38:30 +0000
@@ -772,7 +772,6 @@
      return errors;
    };

-
    /**
     * Utility object that encapsulates Y.Models and keeps their position
     * state within an SVG canvas.

Index: app/views/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-07-15 15:52:04 +0000
+++ app/views/topology/service.js 2013-07-15 19:38:30 +0000
@@ -883,7 +883,9 @@
        node.append('image')
         .classed('service-icon', true)
         .attr({
- 'xlink:href': function(d) {return d.icon;},
+ 'xlink:href': function(d) {
+ return d.icon;
+ },
              width: 96,
              height: 96,
              transform: 'translate(47, 50)'
@@ -899,13 +901,16 @@
          .classed('statusbar', true);

        status_graph.each(function(d) {
- d3.select(this).property('status_bar', new views.StatusBar({
- resize: false,
- width: 160,
- target: this,
- fontSize: 8,
- labels: false
- }).render());
+ if (!d.subordinate) {
+ d3.select(this).property('status_bar',
+ new views.StatusBar({
+ resize: false,
+ width: 160,
+ target: this,
+ fontSize: 8,
+ labels: false
+ }).render());
+ }
        });
        // Manually attach the touchstart event (see method for details)
        node.each(function(data) {
@@ -1149,7 +1154,7 @@
        node.each(function(d) {
          var status_graph = d3.select(this).select('.statusbar');
          var status_bar = status_graph.property('status_bar');
- if (status_bar) {
+ if (status_bar && !d.subordinate) {
            status_bar.update(d.aggregated_status);
          }
        });