Thanks Thiago. You had an excellent catch in there! I'll fix and repropose.
Gary https://codereview.appspot.com/6775058/diff/1/app/views/charm-search.js File app/views/charm-search.js (right): https://codereview.appspot.com/6775058/diff/1/app/views/charm-search.js#newcode31 app/views/charm-search.js:31: parseInt(scrollContainer.getComputedStyle('height'), 10)), On 2012/10/26 15:51:30, thiago.veronezi wrote: > Does node.get('clientHeight') not work for this case? No. They are in fact different, and I need the difference in this case. The height value does not include the margin or padding, and the clientHeight does. (Neither include the scrollbar, if any). I need to figure out what height to set (since clientHeight is read only) and yet I need to take the margin and padding into account, so I need to get the difference. Another approach would be to add the values of margin-top, padding-top, padding-bottom, and margin-bottom, but they are often not in pixels and it is easier this way. I could replace the *previous* line (scrollContainer.getClientRect().height) with offsetHeight. Those are the same as long as the box has scrolling for overflow. I actually like that idea, and might do it. https://codereview.appspot.com/6775058/diff/1/app/views/charm-search.js#newcode597 app/views/charm-search.js:597: svg && parseInt(Y.one('svg').getAttribute('height'), 10) + 17}; On 2012/10/26 15:51:30, thiago.veronezi wrote: > You dont need to call "Y.one" again. You already have the svg object. Correct, thanks. > What happens when we have no svg object? height is undefined, which is fine for the tests. However, your question made me realize that this will not do what I want on interior pages! That was probably your point. Thanks, good catch. I'll fix it. https://codereview.appspot.com/6775058/diff/1/app/views/charm-search.js#newcode655 app/views/charm-search.js:655: // if (sub) { sub.detach(); } On 2012/10/26 15:51:30, thiago.veronezi wrote: > You don't use the sub object. Do you need the loop? Yes. I do use it: I call detach. That was the intent. https://codereview.appspot.com/6775058/ -- https://code.launchpad.net/~gary/juju-gui/charmpanel/+merge/131605 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/charmpanel 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
