Thanks for the review, Thiago. See below for notes.
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'), On 2012/10/19 18:09:29, thiago wrote: > If you are using ids you could simply use Y.one, right? Or you could use a class > instead. I don't think that using a class would buy me anything here. The ambiguous relation menu is unique, and referred to by a unique id. Convention in the past has been to access nodes through container. https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1463 app/views/environment.js:1463: endpoints[m.id].forEach(function(endpoint) { On 2012/10/19 18:09:29, thiago wrote: > 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? Done. I initially couldn't figure out how I'd do it with Handlebars given that the click action references each individual endpoint set, but I did it with node data. 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 { On 2012/10/19 18:09:29, thiago wrote: > You could remove the id and use both classes in your menu. Like... > <div class="service-menu ambiguous-relation-menu"> > <div class="triangle"> </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 Done. Both #service-menu and #ambiguous-relation-menu are unique and must be referred to that way because of the way they're controlled in the view. However, I have added a 'environment-menu' class to each, and then used that as the selector here. 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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
