Code review comment for lp:~gary/juju-gui/exposed

Revision history for this message
Gary Poster (gary) wrote :

Reviewers: mp+160244_code.launchpad.net,

Message:
Please take a look.

Description:
Fix small issue with exposed rendering

If a service was already exposed when the GUI started, trying to
unexpose it would not change the visual representation. This was
because we were rendering multiple instances of the exposed icon per
service block, unnecessarily, and not always removing all of the ones we
needed to. Now we simply only add a single exposed icon at a time.

I plan to self-review.

https://code.launchpad.net/~gary/juju-gui/exposed/+merge/160244

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/views/topology/service.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/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-04-20 03:04:47 +0000
+++ app/views/topology/service.js 2013-04-23 00:57:07 +0000
@@ -875,7 +875,9 @@

        // Show whether or not the service is exposed using an indicator.
        node.filter(function(d) {
- return d.exposed;
+ return d.exposed &&
+ d3.select(this)
+ .select('.exposed-indicator').empty();
        })
          .append('image')
          .attr({'class': 'exposed-indicator on',

« Back to merge proposal