action list in form inheritance

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

action list in form inheritance

Harmeet Bedi
Actions seem to modeled part of form construction. e.g. when i construct a
form at runtime it first runs all the actions and then creates fields.
Sometimes it may create fields based on variables set in action block and
use-when conditions.

Now this construction step breaks down when i have a form inheriting from
another form. If the form does not define any actions, it gets the action
list from it's parent and everything is ok.
But if a form has any actions, even a 'set' for a field it masks all the
actions of the parent form.

In ModelForm
public void initForm(Element formElement) {
...
     if ( parent != null ) {
...
          this.actions = parent.actions;
...
     }
...
   Element actionsElement = UtilXml.firstChildElement(formElement,
"actions");

   if (actionsElement != null) {
       this.actions = ModelFormAction.readSubActions(this, actionsElement);
   }
...

}

I was thinking of form actions as a list of steps that constitute runtime
form construction. It is instead done as an override. This is problematic
and a potential pitfall to me.

I would expect a user  of ofbiz would have this interaction
- They see a form, like it fields and behaviour and then decide to inherit
and extend it to add some value.
- User decides to alter form actions to add some fields and alter some
actions.  Suddenly ancestor form breaks unless all the form actions in each
ancestor are copied or somehow called from leaf form. Now often users will
not know this so will be left scratching their head for hours on why the
form that they liked does not behave as they expected.

To me it would be better to append actions as in:


in ModelForm

public void initForm(Element formElement) {
...
     if ( parent != null ) {
...
          this.actions.appendAll(parent.actions);
...
     }
...
   Element actionsElement = UtilXml.firstChildElement(formElement,
"actions");

   if (actionsElement != null) {
       this.actions.appendAll(ModelFormAction.readSubActions(this,
actionsElement));
   }
...

}

Reasons:
It would be more consistent with inheritance semantics in construction -
that is when constructing a derived object - if constructor in base class
has functions and constructor in derived class has functions, the functions
in base class are run followed by functions in derived class.
Users will inherit and add to form fields and actions. Users would ideally
want to add limited value to base forms and want to keep it's behaviour
unchanged.
It could make form inheritance simpler to get for new users.
I did a walk through of existing ofbiz code. This change does not seem to
adversely impact.


Does this seem sensible. thoughts ?

Harmeet
Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

David E. Jones-2

Harmeet,

The original intent of this design was to be kind of like a method in  
a class in that if you specify a method with the same name in a sub-
class then it will override the method in the parent class so that  
none of that method is run.

The basic problem is what if you didn't want one or more of the  
actions in the parent form to run, how would you do it? Because there  
is no way to specify which if the parent actions you want or don't  
want (unless we did some sort of action blocks with names and you  
could include/exclude them explicitly), the idea is that you either  
use the parent actions (like a parent class method), or you override  
the action block and specify the ones you want to run.

As for my opinion on it, as long as we have a way to get it to not run  
the parent actions (ie for cases when they are not desired or they  
conflict with other things that need to be done in the sub-form) then  
some variation on what it currently does would be fine.

-David


On Aug 30, 2009, at 9:28 PM, Harmeet Bedi wrote:

> Actions seem to modeled part of form construction. e.g. when i  
> construct a
> form at runtime it first runs all the actions and then creates fields.
> Sometimes it may create fields based on variables set in action  
> block and
> use-when conditions.
>
> Now this construction step breaks down when i have a form inheriting  
> from
> another form. If the form does not define any actions, it gets the  
> action
> list from it's parent and everything is ok.
> But if a form has any actions, even a 'set' for a field it masks all  
> the
> actions of the parent form.
>
> In ModelForm
> public void initForm(Element formElement) {
> ...
>     if ( parent != null ) {
> ...
>          this.actions = parent.actions;
> ...
>     }
> ...
>   Element actionsElement = UtilXml.firstChildElement(formElement,
> "actions");
>
>   if (actionsElement != null) {
>       this.actions = ModelFormAction.readSubActions(this,  
> actionsElement);
>   }
> ...
>
> }
>
> I was thinking of form actions as a list of steps that constitute  
> runtime
> form construction. It is instead done as an override. This is  
> problematic
> and a potential pitfall to me.
>
> I would expect a user  of ofbiz would have this interaction
> - They see a form, like it fields and behaviour and then decide to  
> inherit
> and extend it to add some value.
> - User decides to alter form actions to add some fields and alter some
> actions.  Suddenly ancestor form breaks unless all the form actions  
> in each
> ancestor are copied or somehow called from leaf form. Now often  
> users will
> not know this so will be left scratching their head for hours on why  
> the
> form that they liked does not behave as they expected.
>
> To me it would be better to append actions as in:
>
>
> in ModelForm
>
> public void initForm(Element formElement) {
> ...
>     if ( parent != null ) {
> ...
>          this.actions.appendAll(parent.actions);
> ...
>     }
> ...
>   Element actionsElement = UtilXml.firstChildElement(formElement,
> "actions");
>
>   if (actionsElement != null) {
>       this.actions.appendAll(ModelFormAction.readSubActions(this,
> actionsElement));
>   }
> ...
>
> }
>
> Reasons:
> It would be more consistent with inheritance semantics in  
> construction -
> that is when constructing a derived object - if constructor in base  
> class
> has functions and constructor in derived class has functions, the  
> functions
> in base class are run followed by functions in derived class.
> Users will inherit and add to form fields and actions. Users would  
> ideally
> want to add limited value to base forms and want to keep it's  
> behaviour
> unchanged.
> It could make form inheritance simpler to get for new users.
> I did a walk through of existing ofbiz code. This change does not  
> seem to
> adversely impact.
>
>
> Does this seem sensible. thoughts ?
>
> Harmeet

Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

Adrian Crum
Maybe what the actions section needs is an element that would duplicate
super().

-Adrian

David E Jones wrote:

>
> Harmeet,
>
> The original intent of this design was to be kind of like a method in a
> class in that if you specify a method with the same name in a sub-class
> then it will override the method in the parent class so that none of
> that method is run.
>
> The basic problem is what if you didn't want one or more of the actions
> in the parent form to run, how would you do it? Because there is no way
> to specify which if the parent actions you want or don't want (unless we
> did some sort of action blocks with names and you could include/exclude
> them explicitly), the idea is that you either use the parent actions
> (like a parent class method), or you override the action block and
> specify the ones you want to run.
>
> As for my opinion on it, as long as we have a way to get it to not run
> the parent actions (ie for cases when they are not desired or they
> conflict with other things that need to be done in the sub-form) then
> some variation on what it currently does would be fine.
>
> -David
>
>
> On Aug 30, 2009, at 9:28 PM, Harmeet Bedi wrote:
>
>> Actions seem to modeled part of form construction. e.g. when i
>> construct a
>> form at runtime it first runs all the actions and then creates fields.
>> Sometimes it may create fields based on variables set in action block and
>> use-when conditions.
>>
>> Now this construction step breaks down when i have a form inheriting from
>> another form. If the form does not define any actions, it gets the action
>> list from it's parent and everything is ok.
>> But if a form has any actions, even a 'set' for a field it masks all the
>> actions of the parent form.
>>
>> In ModelForm
>> public void initForm(Element formElement) {
>> ...
>>     if ( parent != null ) {
>> ...
>>          this.actions = parent.actions;
>> ...
>>     }
>> ...
>>   Element actionsElement = UtilXml.firstChildElement(formElement,
>> "actions");
>>
>>   if (actionsElement != null) {
>>       this.actions = ModelFormAction.readSubActions(this,
>> actionsElement);
>>   }
>> ...
>>
>> }
>>
>> I was thinking of form actions as a list of steps that constitute runtime
>> form construction. It is instead done as an override. This is problematic
>> and a potential pitfall to me.
>>
>> I would expect a user  of ofbiz would have this interaction
>> - They see a form, like it fields and behaviour and then decide to
>> inherit
>> and extend it to add some value.
>> - User decides to alter form actions to add some fields and alter some
>> actions.  Suddenly ancestor form breaks unless all the form actions in
>> each
>> ancestor are copied or somehow called from leaf form. Now often users
>> will
>> not know this so will be left scratching their head for hours on why the
>> form that they liked does not behave as they expected.
>>
>> To me it would be better to append actions as in:
>>
>>
>> in ModelForm
>>
>> public void initForm(Element formElement) {
>> ...
>>     if ( parent != null ) {
>> ...
>>          this.actions.appendAll(parent.actions);
>> ...
>>     }
>> ...
>>   Element actionsElement = UtilXml.firstChildElement(formElement,
>> "actions");
>>
>>   if (actionsElement != null) {
>>       this.actions.appendAll(ModelFormAction.readSubActions(this,
>> actionsElement));
>>   }
>> ...
>>
>> }
>>
>> Reasons:
>> It would be more consistent with inheritance semantics in construction -
>> that is when constructing a derived object - if constructor in base class
>> has functions and constructor in derived class has functions, the
>> functions
>> in base class are run followed by functions in derived class.
>> Users will inherit and add to form fields and actions. Users would
>> ideally
>> want to add limited value to base forms and want to keep it's behaviour
>> unchanged.
>> It could make form inheritance simpler to get for new users.
>> I did a walk through of existing ofbiz code. This change does not seem to
>> adversely impact.
>>
>>
>> Does this seem sensible. thoughts ?
>>
>> Harmeet
>
>
Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

David E. Jones-2

Yes, that would be another way... ie don't call parent form actions  
unless it is explicitly specific as opposed to calling it unless  
something says not to call it.

-David


On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:

> Maybe what the actions section needs is an element that would  
> duplicate super().
>
> -Adrian
>
> David E Jones wrote:
>> Harmeet,
>> The original intent of this design was to be kind of like a method  
>> in a class in that if you specify a method with the same name in a  
>> sub-class then it will override the method in the parent class so  
>> that none of that method is run.
>> The basic problem is what if you didn't want one or more of the  
>> actions in the parent form to run, how would you do it? Because  
>> there is no way to specify which if the parent actions you want or  
>> don't want (unless we did some sort of action blocks with names and  
>> you could include/exclude them explicitly), the idea is that you  
>> either use the parent actions (like a parent class method), or you  
>> override the action block and specify the ones you want to run.
>> As for my opinion on it, as long as we have a way to get it to not  
>> run the parent actions (ie for cases when they are not desired or  
>> they conflict with other things that need to be done in the sub-
>> form) then some variation on what it currently does would be fine.
>> -David
>> On Aug 30, 2009, at 9:28 PM, Harmeet Bedi wrote:
>>> Actions seem to modeled part of form construction. e.g. when i  
>>> construct a
>>> form at runtime it first runs all the actions and then creates  
>>> fields.
>>> Sometimes it may create fields based on variables set in action  
>>> block and
>>> use-when conditions.
>>>
>>> Now this construction step breaks down when i have a form  
>>> inheriting from
>>> another form. If the form does not define any actions, it gets the  
>>> action
>>> list from it's parent and everything is ok.
>>> But if a form has any actions, even a 'set' for a field it masks  
>>> all the
>>> actions of the parent form.
>>>
>>> In ModelForm
>>> public void initForm(Element formElement) {
>>> ...
>>>    if ( parent != null ) {
>>> ...
>>>         this.actions = parent.actions;
>>> ...
>>>    }
>>> ...
>>>  Element actionsElement = UtilXml.firstChildElement(formElement,
>>> "actions");
>>>
>>>  if (actionsElement != null) {
>>>      this.actions = ModelFormAction.readSubActions(this,  
>>> actionsElement);
>>>  }
>>> ...
>>>
>>> }
>>>
>>> I was thinking of form actions as a list of steps that constitute  
>>> runtime
>>> form construction. It is instead done as an override. This is  
>>> problematic
>>> and a potential pitfall to me.
>>>
>>> I would expect a user  of ofbiz would have this interaction
>>> - They see a form, like it fields and behaviour and then decide to  
>>> inherit
>>> and extend it to add some value.
>>> - User decides to alter form actions to add some fields and alter  
>>> some
>>> actions.  Suddenly ancestor form breaks unless all the form  
>>> actions in each
>>> ancestor are copied or somehow called from leaf form. Now often  
>>> users will
>>> not know this so will be left scratching their head for hours on  
>>> why the
>>> form that they liked does not behave as they expected.
>>>
>>> To me it would be better to append actions as in:
>>>
>>>
>>> in ModelForm
>>>
>>> public void initForm(Element formElement) {
>>> ...
>>>    if ( parent != null ) {
>>> ...
>>>         this.actions.appendAll(parent.actions);
>>> ...
>>>    }
>>> ...
>>>  Element actionsElement = UtilXml.firstChildElement(formElement,
>>> "actions");
>>>
>>>  if (actionsElement != null) {
>>>      this.actions.appendAll(ModelFormAction.readSubActions(this,
>>> actionsElement));
>>>  }
>>> ...
>>>
>>> }
>>>
>>> Reasons:
>>> It would be more consistent with inheritance semantics in  
>>> construction -
>>> that is when constructing a derived object - if constructor in  
>>> base class
>>> has functions and constructor in derived class has functions, the  
>>> functions
>>> in base class are run followed by functions in derived class.
>>> Users will inherit and add to form fields and actions. Users would  
>>> ideally
>>> want to add limited value to base forms and want to keep it's  
>>> behaviour
>>> unchanged.
>>> It could make form inheritance simpler to get for new users.
>>> I did a walk through of existing ofbiz code. This change does not  
>>> seem to
>>> adversely impact.
>>>
>>>
>>> Does this seem sensible. thoughts ?
>>>
>>> Harmeet

Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

Harmeet Bedi
In reply to this post by Harmeet Bedi
To me main main motivation for raising this is that the rules of ofbiz are sometimes hidden for non expert users and current override may be a pitfall.


How about this:
Have 2 attributes to a form with default values.
1. parent-actions with default override and options append(super semantics), override and ignore(for completion). Details documented below
2. parent-row-actions. Similar to (1) for rows.


Since default is override, the behavior will be backwards compatible, defaults ensure no need to change any existing forms, rules are exposed xsd documentation so easier for users to know and system is more flexible.

widget-form.xsd
<xs:attributeGroup name="attlist.form">
...
<xs:attribute name="parent-actions" default="override">
  <xs:annotation>
    <xs:documentation>If form derives from parent, form actions may
      override existing parent form actions, append to parent form actions or ignore
      parent form actions</xs:documentation>
  </xs:annotation>
  <xs:simpleType>
    <xs:restriction base="xs:token">
      <xs:enumeration value="append">
        <xs:annotation>
          <xs:documentation>append form actions to list of inherited parent form actions</xs:documentation>
        </xs:annotation>
      </xs:enumeration>
      <xs:enumeration value="override">
        <xs:annotation>
          <xs:documentation>If action block exists, ignore parent action list.
                            If action block does not exist use the parent action list
          </xs:documentation>
        </xs:annotation>
      </xs:enumeration>
      <xs:enumeration value="ignore">
        <xs:annotation>
          <xs:documentation>Ignore parent form actions.
                            Same as override with no actions specified in actions block.
          </xs:documentation>
        </xs:annotation>
      </xs:enumeration>
    </xs:restriction>
  </xs:simpleType>
</xs:attribute>
...
</xs:attributeGroup>


if ok, will put together patch.

Harmeet


----- Original Message -----
From: "David E Jones" <[hidden email]>
To: [hidden email]
Sent: Monday, August 31, 2009 8:18:00 PM GMT -05:00 US/Canada Eastern
Subject: Re: action list in form inheritance


Yes, that would be another way... ie don't call parent form actions  
unless it is explicitly specific as opposed to calling it unless  
something says not to call it.

-David


On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:

> Maybe what the actions section needs is an element that would  
> duplicate super().
>
> -Adrian
>
> David E Jones wrote:
>> Harmeet,
>> The original intent of this design was to be kind of like a method  
>> in a class in that if you specify a method with the same name in a  
>> sub-class then it will override the method in the parent class so  
>> that none of that method is run.
>> The basic problem is what if you didn't want one or more of the  
>> actions in the parent form to run, how would you do it? Because  
>> there is no way to specify which if the parent actions you want or  
>> don't want (unless we did some sort of action blocks with names and  
>> you could include/exclude them explicitly), the idea is that you  
>> either use the parent actions (like a parent class method), or you  
>> override the action block and specify the ones you want to run.
>> As for my opinion on it, as long as we have a way to get it to not  
>> run the parent actions (ie for cases when they are not desired or  
>> they conflict with other things that need to be done in the sub-
>> form) then some variation on what it currently does would be fine.
>> -David
>> On Aug 30, 2009, at 9:28 PM, Harmeet Bedi wrote:
>>> Actions seem to modeled part of form construction. e.g. when i  
>>> construct a
>>> form at runtime it first runs all the actions and then creates  
>>> fields.
>>> Sometimes it may create fields based on variables set in action  
>>> block and
>>> use-when conditions.
>>>
>>> Now this construction step breaks down when i have a form  
>>> inheriting from
>>> another form. If the form does not define any actions, it gets the  
>>> action
>>> list from it's parent and everything is ok.
>>> But if a form has any actions, even a 'set' for a field it masks  
>>> all the
>>> actions of the parent form.
>>>
>>> In ModelForm
>>> public void initForm(Element formElement) {
>>> ...
>>>    if ( parent != null ) {
>>> ...
>>>         this.actions = parent.actions;
>>> ...
>>>    }
>>> ...
>>>  Element actionsElement = UtilXml.firstChildElement(formElement,
>>> "actions");
>>>
>>>  if (actionsElement != null) {
>>>      this.actions = ModelFormAction.readSubActions(this,  
>>> actionsElement);
>>>  }
>>> ...
>>>
>>> }
>>>
>>> I was thinking of form actions as a list of steps that constitute  
>>> runtime
>>> form construction. It is instead done as an override. This is  
>>> problematic
>>> and a potential pitfall to me.
>>>
>>> I would expect a user  of ofbiz would have this interaction
>>> - They see a form, like it fields and behaviour and then decide to  
>>> inherit
>>> and extend it to add some value.
>>> - User decides to alter form actions to add some fields and alter  
>>> some
>>> actions.  Suddenly ancestor form breaks unless all the form  
>>> actions in each
>>> ancestor are copied or somehow called from leaf form. Now often  
>>> users will
>>> not know this so will be left scratching their head for hours on  
>>> why the
>>> form that they liked does not behave as they expected.
>>>
>>> To me it would be better to append actions as in:
>>>
>>>
>>> in ModelForm
>>>
>>> public void initForm(Element formElement) {
>>> ...
>>>    if ( parent != null ) {
>>> ...
>>>         this.actions.appendAll(parent.actions);
>>> ...
>>>    }
>>> ...
>>>  Element actionsElement = UtilXml.firstChildElement(formElement,
>>> "actions");
>>>
>>>  if (actionsElement != null) {
>>>      this.actions.appendAll(ModelFormAction.readSubActions(this,
>>> actionsElement));
>>>  }
>>> ...
>>>
>>> }
>>>
>>> Reasons:
>>> It would be more consistent with inheritance semantics in  
>>> construction -
>>> that is when constructing a derived object - if constructor in  
>>> base class
>>> has functions and constructor in derived class has functions, the  
>>> functions
>>> in base class are run followed by functions in derived class.
>>> Users will inherit and add to form fields and actions. Users would  
>>> ideally
>>> want to add limited value to base forms and want to keep it's  
>>> behaviour
>>> unchanged.
>>> It could make form inheritance simpler to get for new users.
>>> I did a walk through of existing ofbiz code. This change does not  
>>> seem to
>>> adversely impact.
>>>
>>>
>>> Does this seem sensible. thoughts ?
>>>
>>> Harmeet

Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

David E. Jones-2

Sounds fine to me. One small suggestion: to be more consistent with  
the existing "extends" attribute, maybe we should have something like  
extends-actions.

-David


On Aug 31, 2009, at 9:47 PM, Harmeet Bedi wrote:

> To me main main motivation for raising this is that the rules of  
> ofbiz are sometimes hidden for non expert users and current override  
> may be a pitfall.
>
>
> How about this:
> Have 2 attributes to a form with default values.
> 1. parent-actions with default override and options append(super  
> semantics), override and ignore(for completion). Details documented  
> below
> 2. parent-row-actions. Similar to (1) for rows.
>
>
> Since default is override, the behavior will be backwards  
> compatible, defaults ensure no need to change any existing forms,  
> rules are exposed xsd documentation so easier for users to know and  
> system is more flexible.
>
> widget-form.xsd
> <xs:attributeGroup name="attlist.form">
> ...
> <xs:attribute name="parent-actions" default="override">
>  <xs:annotation>
>    <xs:documentation>If form derives from parent, form actions may
>      override existing parent form actions, append to parent form  
> actions or ignore
>      parent form actions</xs:documentation>
>  </xs:annotation>
>  <xs:simpleType>
>    <xs:restriction base="xs:token">
>      <xs:enumeration value="append">
>        <xs:annotation>
>          <xs:documentation>append form actions to list of inherited  
> parent form actions</xs:documentation>
>        </xs:annotation>
>      </xs:enumeration>
>      <xs:enumeration value="override">
>        <xs:annotation>
>          <xs:documentation>If action block exists, ignore parent  
> action list.
>                            If action block does not exist use the  
> parent action list
>          </xs:documentation>
>        </xs:annotation>
>      </xs:enumeration>
>      <xs:enumeration value="ignore">
>        <xs:annotation>
>          <xs:documentation>Ignore parent form actions.
>                            Same as override with no actions  
> specified in actions block.
>          </xs:documentation>
>        </xs:annotation>
>      </xs:enumeration>
>    </xs:restriction>
>  </xs:simpleType>
> </xs:attribute>
> ...
> </xs:attributeGroup>
>
>
> if ok, will put together patch.
>
> Harmeet
>
>
> ----- Original Message -----
> From: "David E Jones" <[hidden email]>
> To: [hidden email]
> Sent: Monday, August 31, 2009 8:18:00 PM GMT -05:00 US/Canada Eastern
> Subject: Re: action list in form inheritance
>
>
> Yes, that would be another way... ie don't call parent form actions
> unless it is explicitly specific as opposed to calling it unless
> something says not to call it.
>
> -David
>
>
> On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:
>
>> Maybe what the actions section needs is an element that would
>> duplicate super().
>>
>> -Adrian
>>
>> David E Jones wrote:
>>> Harmeet,
>>> The original intent of this design was to be kind of like a method
>>> in a class in that if you specify a method with the same name in a
>>> sub-class then it will override the method in the parent class so
>>> that none of that method is run.
>>> The basic problem is what if you didn't want one or more of the
>>> actions in the parent form to run, how would you do it? Because
>>> there is no way to specify which if the parent actions you want or
>>> don't want (unless we did some sort of action blocks with names and
>>> you could include/exclude them explicitly), the idea is that you
>>> either use the parent actions (like a parent class method), or you
>>> override the action block and specify the ones you want to run.
>>> As for my opinion on it, as long as we have a way to get it to not
>>> run the parent actions (ie for cases when they are not desired or
>>> they conflict with other things that need to be done in the sub-
>>> form) then some variation on what it currently does would be fine.
>>> -David
>>> On Aug 30, 2009, at 9:28 PM, Harmeet Bedi wrote:
>>>> Actions seem to modeled part of form construction. e.g. when i
>>>> construct a
>>>> form at runtime it first runs all the actions and then creates
>>>> fields.
>>>> Sometimes it may create fields based on variables set in action
>>>> block and
>>>> use-when conditions.
>>>>
>>>> Now this construction step breaks down when i have a form
>>>> inheriting from
>>>> another form. If the form does not define any actions, it gets the
>>>> action
>>>> list from it's parent and everything is ok.
>>>> But if a form has any actions, even a 'set' for a field it masks
>>>> all the
>>>> actions of the parent form.
>>>>
>>>> In ModelForm
>>>> public void initForm(Element formElement) {
>>>> ...
>>>>   if ( parent != null ) {
>>>> ...
>>>>        this.actions = parent.actions;
>>>> ...
>>>>   }
>>>> ...
>>>> Element actionsElement = UtilXml.firstChildElement(formElement,
>>>> "actions");
>>>>
>>>> if (actionsElement != null) {
>>>>     this.actions = ModelFormAction.readSubActions(this,
>>>> actionsElement);
>>>> }
>>>> ...
>>>>
>>>> }
>>>>
>>>> I was thinking of form actions as a list of steps that constitute
>>>> runtime
>>>> form construction. It is instead done as an override. This is
>>>> problematic
>>>> and a potential pitfall to me.
>>>>
>>>> I would expect a user  of ofbiz would have this interaction
>>>> - They see a form, like it fields and behaviour and then decide to
>>>> inherit
>>>> and extend it to add some value.
>>>> - User decides to alter form actions to add some fields and alter
>>>> some
>>>> actions.  Suddenly ancestor form breaks unless all the form
>>>> actions in each
>>>> ancestor are copied or somehow called from leaf form. Now often
>>>> users will
>>>> not know this so will be left scratching their head for hours on
>>>> why the
>>>> form that they liked does not behave as they expected.
>>>>
>>>> To me it would be better to append actions as in:
>>>>
>>>>
>>>> in ModelForm
>>>>
>>>> public void initForm(Element formElement) {
>>>> ...
>>>>   if ( parent != null ) {
>>>> ...
>>>>        this.actions.appendAll(parent.actions);
>>>> ...
>>>>   }
>>>> ...
>>>> Element actionsElement = UtilXml.firstChildElement(formElement,
>>>> "actions");
>>>>
>>>> if (actionsElement != null) {
>>>>     this.actions.appendAll(ModelFormAction.readSubActions(this,
>>>> actionsElement));
>>>> }
>>>> ...
>>>>
>>>> }
>>>>
>>>> Reasons:
>>>> It would be more consistent with inheritance semantics in
>>>> construction -
>>>> that is when constructing a derived object - if constructor in
>>>> base class
>>>> has functions and constructor in derived class has functions, the
>>>> functions
>>>> in base class are run followed by functions in derived class.
>>>> Users will inherit and add to form fields and actions. Users would
>>>> ideally
>>>> want to add limited value to base forms and want to keep it's
>>>> behaviour
>>>> unchanged.
>>>> It could make form inheritance simpler to get for new users.
>>>> I did a walk through of existing ofbiz code. This change does not
>>>> seem to
>>>> adversely impact.
>>>>
>>>>
>>>> Does this seem sensible. thoughts ?
>>>>
>>>> Harmeet
>

Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

Bob Morley
In reply to this post by David E. Jones-2
I really like the super idea.  Here is a proposal:

- add support for the "super" element (or some reasonable name) to both actions and row-actions
- add a new attribute "override" to the actions and row-actions.  Its only value would be "true" (it would be nice to have attribute minimization here so we could have no attribute value)
- if the "override" attribute is not included on an actions or row-actions block that has a parent, a warning should be produced to the developer informing them they should add that attribute

Reasons:
- most flexibility
- most visibility
- does no harm (unless you count warning messages :) )

The thing I do not like about the current approach is that you are going to bury more attributes on an already bloated form.  The new attributes are going to have defaults (for backwards compat) so no one is going to set them anyway.  These attributes, if used, should be on the rows and row-actions elements IMHO (but I prefer the proposal above).

David E Jones-4 wrote
Yes, that would be another way... ie don't call parent form actions  
unless it is explicitly specific as opposed to calling it unless  
something says not to call it.

-David


On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:

> Maybe what the actions section needs is an element that would  
> duplicate super().
>
> -Adrian
>
Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

Harmeet Bedi
You have to look at form attributes to see how to extend. So seeing related extends attributes - extends-actions and extends-row-actions may help a person understand potential options with extend.

I feel goal should be to help new users become more efficient and make behavior assumptions more visible. I feel 'super' element may help power users but does not reduce curve.

Harmeet


----- Original Message -----
From: "Bob Morley" <[hidden email]>
To: [hidden email]
Sent: Tuesday, September 1, 2009 10:03:01 AM GMT -05:00 US/Canada Eastern
Subject: Re: action list in form inheritance


I really like the super idea.  Here is a proposal:

- add support for the "super" element (or some reasonable name) to both
actions and row-actions
- add a new attribute "override" to the actions and row-actions.  Its only
value would be "true" (it would be nice to have attribute minimization here
so we could have no attribute value)
- if the "override" attribute is not included on an actions or row-actions
block that has a parent, a warning should be produced to the developer
informing them they should add that attribute

Reasons:
- most flexibility
- most visibility
- does no harm (unless you count warning messages :) )

The thing I do not like about the current approach is that you are going to
bury more attributes on an already bloated form.  The new attributes are
going to have defaults (for backwards compat) so no one is going to set them
anyway.  These attributes, if used, should be on the rows and row-actions
elements IMHO (but I prefer the proposal above).


David E Jones-4 wrote:

>
>
> Yes, that would be another way... ie don't call parent form actions  
> unless it is explicitly specific as opposed to calling it unless  
> something says not to call it.
>
> -David
>
>
> On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:
>
>> Maybe what the actions section needs is an element that would  
>> duplicate super().
>>
>> -Adrian
>>
>

--
View this message in context: http://www.nabble.com/action-list-in-form-inheritance-tp25217949p25240756.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.

Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

David E. Jones-2

I'll admit that this is fairly subjective, and one can guess at what  
most people would find intuitive, but as for my subjectivity it says  
that additional extends-* attributes would be better. On the other  
hand, there are a lot of attributes on the form elements, but I guess  
that's another topic really...

-David


On Sep 1, 2009, at 11:53 AM, Harmeet Bedi wrote:

> You have to look at form attributes to see how to extend. So seeing  
> related extends attributes - extends-actions and extends-row-actions  
> may help a person understand potential options with extend.
>
> I feel goal should be to help new users become more efficient and  
> make behavior assumptions more visible. I feel 'super' element may  
> help power users but does not reduce curve.
>
> Harmeet
>
>
> ----- Original Message -----
> From: "Bob Morley" <[hidden email]>
> To: [hidden email]
> Sent: Tuesday, September 1, 2009 10:03:01 AM GMT -05:00 US/Canada  
> Eastern
> Subject: Re: action list in form inheritance
>
>
> I really like the super idea.  Here is a proposal:
>
> - add support for the "super" element (or some reasonable name) to  
> both
> actions and row-actions
> - add a new attribute "override" to the actions and row-actions.  
> Its only
> value would be "true" (it would be nice to have attribute  
> minimization here
> so we could have no attribute value)
> - if the "override" attribute is not included on an actions or row-
> actions
> block that has a parent, a warning should be produced to the developer
> informing them they should add that attribute
>
> Reasons:
> - most flexibility
> - most visibility
> - does no harm (unless you count warning messages :) )
>
> The thing I do not like about the current approach is that you are  
> going to
> bury more attributes on an already bloated form.  The new attributes  
> are
> going to have defaults (for backwards compat) so no one is going to  
> set them
> anyway.  These attributes, if used, should be on the rows and row-
> actions
> elements IMHO (but I prefer the proposal above).
>
>
> David E Jones-4 wrote:
>>
>>
>> Yes, that would be another way... ie don't call parent form actions
>> unless it is explicitly specific as opposed to calling it unless
>> something says not to call it.
>>
>> -David
>>
>>
>> On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:
>>
>>> Maybe what the actions section needs is an element that would
>>> duplicate super().
>>>
>>> -Adrian
>>>
>>
>
> --
> View this message in context: http://www.nabble.com/action-list-in-form-inheritance-tp25217949p25240756.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

Adrian Crum
One of the nice things about an element is you can place it anywhere in
the child form's actions. In other words, run some actions, call parent
actions, run some more actions.

-Adrian

David E Jones wrote:

>
> I'll admit that this is fairly subjective, and one can guess at what
> most people would find intuitive, but as for my subjectivity it says
> that additional extends-* attributes would be better. On the other hand,
> there are a lot of attributes on the form elements, but I guess that's
> another topic really...
>
> -David
>
>
> On Sep 1, 2009, at 11:53 AM, Harmeet Bedi wrote:
>
>> You have to look at form attributes to see how to extend. So seeing
>> related extends attributes - extends-actions and extends-row-actions
>> may help a person understand potential options with extend.
>>
>> I feel goal should be to help new users become more efficient and make
>> behavior assumptions more visible. I feel 'super' element may help
>> power users but does not reduce curve.
>>
>> Harmeet
>>
>>
>> ----- Original Message -----
>> From: "Bob Morley" <[hidden email]>
>> To: [hidden email]
>> Sent: Tuesday, September 1, 2009 10:03:01 AM GMT -05:00 US/Canada Eastern
>> Subject: Re: action list in form inheritance
>>
>>
>> I really like the super idea.  Here is a proposal:
>>
>> - add support for the "super" element (or some reasonable name) to both
>> actions and row-actions
>> - add a new attribute "override" to the actions and row-actions.  Its
>> only
>> value would be "true" (it would be nice to have attribute minimization
>> here
>> so we could have no attribute value)
>> - if the "override" attribute is not included on an actions or
>> row-actions
>> block that has a parent, a warning should be produced to the developer
>> informing them they should add that attribute
>>
>> Reasons:
>> - most flexibility
>> - most visibility
>> - does no harm (unless you count warning messages :) )
>>
>> The thing I do not like about the current approach is that you are
>> going to
>> bury more attributes on an already bloated form.  The new attributes are
>> going to have defaults (for backwards compat) so no one is going to
>> set them
>> anyway.  These attributes, if used, should be on the rows and row-actions
>> elements IMHO (but I prefer the proposal above).
>>
>>
>> David E Jones-4 wrote:
>>>
>>>
>>> Yes, that would be another way... ie don't call parent form actions
>>> unless it is explicitly specific as opposed to calling it unless
>>> something says not to call it.
>>>
>>> -David
>>>
>>>
>>> On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:
>>>
>>>> Maybe what the actions section needs is an element that would
>>>> duplicate super().
>>>>
>>>> -Adrian
>>>>
>>>
>>
>> --
>> View this message in context:
>> http://www.nabble.com/action-list-in-form-inheritance-tp25217949p25240756.html 
>>
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: action list in form inheritance

Harmeet Bedi
In reply to this post by Harmeet Bedi
I added  patch for attributes to jira ticket : https://issues.apache.org/jira/browse/OFBIZ-2889
Please review when you get a chance.

I added prepend option to mirror append option earlier. This would allow child to do somethings and then hand over to parent actions.
                      <xs:enumeration value="prepend">
                        <xs:annotation>
                          <xs:documentation>prepend form actions to list of parent form actions</xs:documentation>
                        </xs:annotation>


'super' element is certainly powerful. One can also call super multiple times and all the nice goodies. I think power user and non power user needs need not be exlusive. One could develop 'super' element and do custom calling in leaf form. Default 'override' strategy would not come in the way. To me it is still power user and so did not build it up myself.


Harmeet
----- Original Message -----
From: "Adrian Crum" <[hidden email]>
To: [hidden email]
Sent: Tuesday, September 1, 2009 3:38:46 PM GMT -05:00 US/Canada Eastern
Subject: Re: action list in form inheritance

One of the nice things about an element is you can place it anywhere in
the child form's actions. In other words, run some actions, call parent
actions, run some more actions.

-Adrian

David E Jones wrote:

>
> I'll admit that this is fairly subjective, and one can guess at what
> most people would find intuitive, but as for my subjectivity it says
> that additional extends-* attributes would be better. On the other hand,
> there are a lot of attributes on the form elements, but I guess that's
> another topic really...
>
> -David
>
>
> On Sep 1, 2009, at 11:53 AM, Harmeet Bedi wrote:
>
>> You have to look at form attributes to see how to extend. So seeing
>> related extends attributes - extends-actions and extends-row-actions
>> may help a person understand potential options with extend.
>>
>> I feel goal should be to help new users become more efficient and make
>> behavior assumptions more visible. I feel 'super' element may help
>> power users but does not reduce curve.
>>
>> Harmeet
>>
>>
>> ----- Original Message -----
>> From: "Bob Morley" <[hidden email]>
>> To: [hidden email]
>> Sent: Tuesday, September 1, 2009 10:03:01 AM GMT -05:00 US/Canada Eastern
>> Subject: Re: action list in form inheritance
>>
>>
>> I really like the super idea.  Here is a proposal:
>>
>> - add support for the "super" element (or some reasonable name) to both
>> actions and row-actions
>> - add a new attribute "override" to the actions and row-actions.  Its
>> only
>> value would be "true" (it would be nice to have attribute minimization
>> here
>> so we could have no attribute value)
>> - if the "override" attribute is not included on an actions or
>> row-actions
>> block that has a parent, a warning should be produced to the developer
>> informing them they should add that attribute
>>
>> Reasons:
>> - most flexibility
>> - most visibility
>> - does no harm (unless you count warning messages :) )
>>
>> The thing I do not like about the current approach is that you are
>> going to
>> bury more attributes on an already bloated form.  The new attributes are
>> going to have defaults (for backwards compat) so no one is going to
>> set them
>> anyway.  These attributes, if used, should be on the rows and row-actions
>> elements IMHO (but I prefer the proposal above).
>>
>>
>> David E Jones-4 wrote:
>>>
>>>
>>> Yes, that would be another way... ie don't call parent form actions
>>> unless it is explicitly specific as opposed to calling it unless
>>> something says not to call it.
>>>
>>> -David
>>>
>>>
>>> On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:
>>>
>>>> Maybe what the actions section needs is an element that would
>>>> duplicate super().
>>>>
>>>> -Adrian
>>>>
>>>
>>
>> --
>> View this message in context:
>> http://www.nabble.com/action-list-in-form-inheritance-tp25217949p25240756.html 
>>
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>
>
>