One quibble:  I think `N | 0` is more accepted than `N >> 0` as a way to coerce 
a number to an integer.  At least that's what I see in other code bases.

One comment:  Please add a comment on lzinputtextcontainer explaining why there 
is _not_ padding there.

Otherwise approved.  This moves the padding "inside" the container dimension, 
which was the original intent, except for my confusion over the box model.  It 
might be worthwhile to have a longer comment where you removed the padding for 
measurement, explaining how when setting the height of a text sprite the 
container height wants to include the padding/gutter, but that when measuring 
text you want the accurate height of the text.

On 2010-09-03, at 21:35, Max Carlson wrote:

> Change maxcarlson-20100903-cin by maxcarl...@friendly on 2010-09-03 18:28:08 
> PDT
>    in /Users/maxcarlson/openlaszlo/trunk-clean
>    for http://svn.openlaszlo.org/openlaszlo/trunk
> 
> Summary: Fix textfield sizing in dhtml
> 
> Bugs Fixed: LPP-9268 - DHTML: padding is not subtracted from width/height for 
> texts
> 
> Technical Reviewer: [email protected], ptw
> QA Reviewer: hminsky
> 
> Details: LzSprite - Style text containers with 0px padding, use padding on 
> elements in containers instead.  parseInt() -> >> 0 for a slight speed boost.
> 
> LzTextSprite - Text measurement divs are set to 0px padding to match the 
> style in LzSprite.  Uncomment text padding subtraction in setHeight - the 
> scrolldiv shouldn't include padding.
> 
> Tests: See test/text/newtextformat.lzx?debug=true&lzr=dhtml matches swf8 and 
> test/lztest/lztest-textheight.lzx?lzr=dhtml runs like before.
> 
> Files:
> M       WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js
> M       WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js
> 
> Changeset: 
> http://svn.openlaszlo.org/openlaszlo/patches/maxcarlson-20100903-cin.tar
> 


Reply via email to