[Laszlo-dev] For Review: Change 20080902-Philip-4. Summary: Modify components to use setters tag

[P T Withington]

1) Could we correct this comment/rename the flag?

+/** true forces next call to setAttribute('x',...) to run
+    @access private */
+var _textdirty:Boolean = false;

I would say something like 'forces $lzc$set_text to do a full update,  
typically because the font or style has changed even though the  
content has not'.  The name 'textdirty' is misleading, the text is  
actually fine, but the optimization to not recompute when the content  
doesn't changed is invalid.  Perhaps the flag should be called  
cacheInvalid or something.

2) I think that in the LFC (and only in the LFC, not in components),  
the places where you have replaced `setFoo` with `setAttribute('foo'`  
you should use `$lzc$set_foo`, just for efficiency/compactness.  The  
setAttribute call is correct, but will expand into unnecessary code at  
the moment.

3) I think it is important that there be some correlation between the  
declaration of an attribute and the definition of its setter.  This is  
why I put the original $lzc$set_* trampolines right below the var *  
declaration.  I know this goes against some other design rule where  
all your var's are supposed to be together and then all your methods.   
Maybe at least have a comment for the var's that have setters?   
Because, eventually, when we go to "real" setters, the var's will have  
to become getters and the state will have to be stored in an internal  
var instead, so we'll need to be able to easily find all the var/ 
setter pairs.

4) When there is documentation on an old setFoo that you are  
deprecating, you need to move that documentation to the var foo  
declaration, if applicable.  For example, the layout, height, and  
width attributes.

5) A setter-method will never get more than one argument, so you  
should remove the optional argument declarations when you reformulate  
them (and make sure they were unused!).  E.g., there are setters for  
'x' and 'y' that have optional 'force' arguments.

6) States are not supposed to need explicit 'this.'s, so I'm wondering  
if you _had_ to introduce those?  Maybe this was a mis-communication  
on my part?

---

Otherwise, approved!

On 2008-09-02, at 13:03EDT, Philip Romanik wrote:

> Change 20080902-Philip-4 by Philip at Philip-DC on 2008-09-02 10:55:41  
> EDT
>    in /cygdrive/f/laszlo/svn/src/svn/openlaszlo/trunk
>    for http://svn.openlaszlo.org/openlaszlo/trunk
>
> Summary: Modify components to use setters tag
>
> New Features:
>
> Bugs Fixed: LPP-5644 (partial)
>
> Technical Reviewer: ptw
> QA Reviewer: (pending)
> Doc Reviewer: (pending)
>
> Documentation:
>
> Release Notes:
>
> Details:
> I modified the components to use the setters tag. I also modified  
> the old setter and $lzc_$set_* methods in view/canvas/text/ 
> inputtext. I added an explicit this to some of the states in the  
> components (form, gridcolumn, modaldialog, window, windowpanel).  
> Some of the components needs extra work to get the desired behavior  
> of bgcolor (windowpanel, list, tabslider, tabelement).
>
> Once this changeset is approved I will modify the setters in the  
> rest of the LFC, and start converting the components and LFC to use  
> setAttribute instead of set*. Until then, running with debugging on  
> will generate a lot of deprecated warnings,
>
> Tests:
>  /test/smoke/smokecheck.lzx in swf8/dhtml
>  componentsampler, list, grid all work in swf8/swf9/dhtml.
>  lzpix/amazon runs in swf8/swf9/dhtml
>
> Files:
> M      WEB-INF/lps/lfc/views/LzInputText.lzs
> M      WEB-INF/lps/lfc/views/LzText.lzs
> M      WEB-INF/lps/lfc/views/LaszloView.lzs
> M      WEB-INF/lps/lfc/views/LaszloCanvas.lzs
> M      lps/components/debugger/debugger.lzx
> M      lps/components/debugger/newcontent.lzx
> M      lps/components/debugger/scrollingtext.lzx
> M      lps/components/lz/form.lzx
> M      lps/components/lz/gridcolumn.lzx
> M      lps/components/lz/tabslider.lzx
> M      lps/components/lz/modaldialog.lzx
> M      lps/components/lz/tabelement.lzx
> M      lps/components/lz/plainfloatinglist.lzx
> M      lps/components/lz/floatinglist.lzx
> M      lps/components/lz/window.lzx
> M      lps/components/lz/list.lzx
> M      lps/components/lz/windowpanel.lzx
> M      lps/components/incubator/rich-text/formatfontcolor.lzx
> M      lps/components/incubator/borderinput.lzx
> M      lps/components/extensions/av/videoview.lzx
> M      lps/components/base/basebutton.lzx
> M      lps/components/base/swatchview.lzx
> M      lps/components/base/baselistitem.lzx
> M      lps/components/base/basewindow.lzx
> M      lps/components/base/basecombobox.lzx
> M      lps/components/base/basetabslider.lzx
> M      lps/components/base/basetabs.lzx
>
> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080902-Philip-4.tar
>