Thanks
https://codereview.appspot.com/6826057/diff/3001/app/modules.js File app/modules.js (right): https://codereview.appspot.com/6826057/diff/3001/app/modules.js#newcode14 app/modules.js:14: 'fullpath': '/juju-ui/assets/javascripts/d3.v2.js' Left over from profiling; back to minified. On 2012/11/09 21:02:40, gary.poster wrote: > I guess this is good for thiago's branch, which will be minifying everything > anyway, so that the debug story is nice. Is that why you did it? https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js File app/views/environment.js (right): https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#newcode645 app/views/environment.js:645: // .one('text').getClientRect() || {width: 0}).width + 10; Unfortunately not, as we use this attribute on line 649, expecting it to be pixels, as we don't know how wide an em is. On 2012/11/09 18:38:31, benjamin.saller wrote: > Can we specify it as d.display_name.length + 10 + 'em'? I didn't test it, but em > should be font size relative if it works. https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#newcode645 app/views/environment.js:645: // .one('text').getClientRect() || {width: 0}).width + 10; Ooops! Done! On 2012/11/09 21:02:40, gary.poster wrote: > I'm -1 on checking in commented-out code to trunk; if you add an explanatory > comment as to why you have left this in but commented out, I might feel better > about it. :-) https://codereview.appspot.com/6826057/ -- https://code.launchpad.net/~makyo/juju-gui/replace-service-module/+merge/132938 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/replace-service-module 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
