yellow team mailing list archive

[Thiago Veronezi <thiago@xxxxxxxxxxxx>]

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1449
app/views/environment.js:1449: var menu =
container.one('#ambiguous-relation-menu'),
If you are using ids you could simply use Y.one, right? Or you could use
a class instead.

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1463
app/views/environment.js:1463:
endpoints[m.id].forEach(function(endpoint) {
You could use handlebars to build the list for you. For example:

//remove old list, if any
menu.all('ul').remove();
.
.
.
//create new <ul><li></li>...<ul>  list
var list = Templates.overviewRelationMenuList({
   endPoints: endpoints
});

list.all('li').on('click', ...

menu.append(list);
.
.
.

This way you remove some html elements from this js file. wdyt?

https://codereview.appspot.com/6736051/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6736051/diff/1/lib/views/stylesheet.less#newcode145
lib/views/stylesheet.less:145: #service-menu, #ambiguous-relation-menu {
You could remove the id and use both classes in your menu. Like...

<div class="service-menu ambiguous-relation-menu">
   <div class="triangle">&nbsp;</div>
     <ul/>
   </div>
</div>

So you can remove this new css declaration and do something like...

var menu = container.one('.ambiguous-relation-menu')

...in environment.js

https://codereview.appspot.com/6736051/

-- 
https://code.launchpad.net/~makyo/juju-gui/ambiguous-relations/+merge/130616
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/ambiguous-relations into lp:juju-gui.