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

[Philip Romanik]

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
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.openlaszlo.org/pipermail/laszlo-dev/attachments/20080903/769910c0/attachment.html