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

[P T Withington]

Great, thanks!

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

> Hi Tucker,
>
> I'll address 1-5. For #3, I'll move the setter below the var and  
> leave the old setters where they are.
> For 6, the explicit this is not necessary to run. I'll see if I can  
> back them out.
>
>
>> 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>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 
>> >http://svn.openlaszlo.org/openlaszlo/patches/20080902-Philip-4.tar
>> >