yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

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.