This is my 30,000-foot take, from memory, not from looking closely at the code:

I think the intent is that you can set a default value into the component, for 
a form say, so that you can 'reset' the form to that default value.  I think 
this is separate from the init-order issues, but probably all balled up with 
it.  I'm pretty sure this was some contributed code.

In any case, it seems to me that we should separate out the two issues:

  1) Provide a way to give a default (rollback) value to a field.

  2) Deal with the initialization-order issues.

Proposal:

  1) Make setText/setValue private.

  2) Add a virtual `default` attribute that calls the private methods above 
with isinitvalue=true

  3) Add normal setters for `text` and `value` that call the private methods 
above with isinitvalue=false

Yeah, I realize this is a huge API change, so it would need to be phased in.  
Hopefully our new components can start fresh and not have this baroque 
interface.

On 2010-07-30, at 20:30, Henry Minsky wrote:

> On Fri, Jul 30, 2010 at 7:42 PM, Max Carlson <[email protected]> wrote:
> 
>> [adding laszlo-dev]
>> 
>> It seems like we should follow the same pattern we do for LzView, etc. -
>> add setters and deprecate the setText() calls for a release (so we can
>> remove them) and have setText() warn and call the setter.
>> 
>> As for the setText() with two args, that can't really be a setter in the
>> current regime so... do we want to allow setters to have more than one
>> argument?
>> 
> 
> I really think we do not want to do that. Unfortunately this second arg  is
> all over the place. I guess
> I don't don't understand what this "isinitvalue" idiom is being used for.
> 
> I see that there's this setValue method on baseformitem:
> 
>        <!--- Set the value for the baseformitem.
>              @param Any v: the value of the baseformitem.
>              @param Boolean isinitvalue: if true, the rollbackvalue is set.
> -->
>        <method name="setValue" args="v,isinitvalue=null"><![CDATA[
>            var didchange = (this.value != v);
>            this.value = v;
>            if (isinitvalue || ! this._initcomplete) {
>                this.rollbackvalue = v;
>            }
>            this.setChanged(didchange && !isinitvalue && this.rollbackvalue
> != v);
>            if (this['onvalue']) this.onvalue.sendEvent(v);
>            ]]>
>        </method>
> 
> So the 'isinitvalue' flag seems to be used to declare that "yes I'm setting
> a value on this component, but no, don't act like the user actually changed
> it" , plus it's setting this "rollback" value.
> 
> I'd love to figure out a better way to do this, but I'm not sure how. In
> some cases, maybe some of these places where 'isinitvalue' is being passed
> could instead be checking the value of LzNode.isinited? If the
> setter is being run during node init, then could we assume that the user
> wants "isinitvalue" to be regarded as true? E.g., they set some of these
> values in the node's initial attribute list
> 
>    <edittext text="this is in initvalue, I assume" />
> 
> Or else we need to have basecomponent define it's own "I am in the
> initialization phase" flag?
> 
> It just seems like there's no compatible way to overload real LZX setters
> with this extra arg.
> 
> 
> 
>> There must be a way to make baseedittext more sane - it seems like all the
>> machinery is there to ensure you can call getValue() before the child
>> textfield is actually initted.  There's a lot of janky stuff going on, e.g.
>> the constructor in _newinternalinputtext placing itself in its parent's
>> _field slot.
>> 
>> I'd start by adding a setters for each method, and have that accumulate
>> values so that when the child inits you can set them all at once.  This is
>> using oldstyle inputtext APIs which need to be deprecated - I'd be surprised
>> if you didn't see warnings with the debugger on...
>> 
>> Once you have the setters in place, it should be much easier to clean up.
>> 
>> Regards,
>> Max Carlson
>> OpenLaszlo.org
>> 
>> 
>> On 7/30/10 10:13 AM, Henry Minsky wrote:
>> 
>>> 
>>> Regarding LPP-9227, "Replace setPattern, setText(t,isinitvalue) with
>>> setAttribute('xxx',xxx) format"
>>> (http://jira.openlaszlo.org/jira/browse/LPP-9227)
>>> 
>>> There's a method in baseedittext.lzx
>>> 
>>> <method name="setPattern" args="r">
>>>            this.setAttribute('pattern', r);
>>>            this._field.setAttribute('pattern', r);
>>> </method>
>>> 
>>> I am thinking I can just change the setPattern method to a setter:
>>> 
>>> <!-- Set the characters which can be entered into a text field. -->
>>> <setter name="pattern" args="r">
>>>            super.setAttribute('pattern', r);
>>>            this._field.setAttribute('pattern', r);
>>> </setter>
>>> 
>>> 
>>> But they also have their own version of setText
>>> 
>>> <method name="setText" args="t,isinitvalue">
>>>            this.doSetValue(t,isinitvalue, false);
>>> </method>
>>> 
>>> This method takes two args. So I think that they can't use a standard
>>> setter method for this, because that is not compliant with the setter
>>> API. I'm not sure who the callers are going to be for this, but it
>>> seems like a new method is needed, with some name besides setText.
>>> 
>>> --
>>> Henry Minsky
>>> Software Architect
>>> [email protected] <mailto:[email protected]>
>>> 
>>> 
>>> 
> 
> 
> -- 
> Henry Minsky
> Software Architect
> [email protected]


Reply via email to