yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

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.