Amy Muntz - 31/May/06 10:58 AM Ben - please wait until we have a contributor agreement in place to do the review. Thanks, Amy.

Daniel Salama - 31/May/06 01:11 PM This version corrects a minor "bug". If the container holding a roundrect is resized, the roundrect was not redrawing itself. I added handlers for onwidth and onheight.

Daniel Salama - 31/May/06 01:31 PM I believe there is a bug that was "included" in the original version and might have just worsen with my changes. Evaluating this code: <roundrect name="test" width="400" height="50" inset="10">   <simplelayout axis="x" spacing="5" />   <text text="Code" />   <edittext name="code" width="50" />   <text text="Name" />   <edittext name="name" width="200" />   <button name="Send" width="50" y="10" options="ignorelayout" align="right" /> </roundrect> The problem lays in the align="right" of the button. In the original version of roundrect, the button will be drawn so that the right edge of the button is _over_ the border line of the roundrect. What I assume should happen is that align="right" should query the width of its container, which in this case is roundrect. Roundrect should report back the width _minus_ the bordersize attribute. With the additions that I made, Roundrect should report back the width _minus_ the bordersize attribute _minus_ insetleft and insetright attributes. Or something along those lines. Does it make sense what I'm saying? Thanks, Daniel

Daniel Salama - 31/May/06 01:38 PM BTW, the y="10" shouldn't be necessary either for it should respect the insettop value of roundrect. Unfortunately, that doesn't work either. Just as an FYI, what I ended up doing was: <roundrect name="test" width="400" height="50" inset="10">   <simplelayout axis="x" spacing="5" />   <text text="Code" />   <edittext name="code" width="50" />   <text text="Name" />   <edittext name="name" width="200" />   <button name="Send" width="50" options="ignorelayout"                  y="${parent.insettop}"                  x="${immediateparent.width - parent.insetright - 50}" /> </roundrect> It does what I want but it's definitely not elegant and defeats the purpose trusting components (e.g. roundrect) Thanks, Daniel

Amy Muntz - 31/May/06 02:34 PM Contributor agreement received. Ben - please proceed with review. Thanks.

Jim Grandy - 20/Jun/06 12:38 AM Hi Daniel, Some review comments for you. 1- In general, diffs are preferable to whole files. 2- You should test your sample cases (comments above) in 3.3.1. Roundrect has a default layout in the y axis (not sure this is the best case), and a regression in 3.3 gives different simplelayout behavior than 3.2 or 3.3.1. 3- It would be great if you integrated your sample code into roundrect-test.lzx in the incubator test directory (lps/components/incubator/test/roundrect-test.lzx) 4- Please use the new event syntax instead of declaring events as attributes. 5- In my testing, it didn't appear as though this.content was set up when this.content.reset() was called in drawStructure from oninit. Run your sample with debug=true to see the debugger warning. You'll also notice that placing a subview in the roundrect with width="${parent.width}" height="${parent.height}" doesn't work -- width and height are instead the default.

Josh Crowley - 03/Apr/07 10:36 PM Approved by Max.