You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by Andreas Andreou <an...@gmail.com> on 2011/08/16 22:38:11 UTC
Radio and RadioGroup as AbstractField subclasses
Hi,
I've started work on TAP5-1602 (Add Zone parameter to Checkbox component)
and it's been an interesting learning experience (it's really the
first time i'm going
over such big part of the T5 codebase) - so, keep that in mind, it
should explain
my ignorance in a few cases!
So, anyway, i'd like to provide the zone parameter to all
AbstractField subclasses
since i don't see the need to special-case it to a particular component
and in fact i've gotten it to work quite nicely.
But (there's always a but!), I then noticed that Radio and RadioGroup do not
extend from AbstractField, So, I'm wondering, are there any particular
reason for this?
Because i see a lot of the AbstractField code being duplicated in
those components
(esp. in RadioGroup).
I've prepared a diff file that would turn those components into
AbstractFields and it's at
https://gist.github.com/ef2fe45d2396c7c3f6a7
It's mainly deleted code (i think close to 150 lines less)! but (i did
mention there's always a
but, didn't I?) it requires a slight change in AbstractField (the
isDisabled method needs to
lose its final modifier since Radio needs to overwrite it). Is that an
acceptable change?
If that's not the case, there's still an alternative that i could be
happy with and that is
to have only RadioGroup extend from AbstractField (which makes some
sense since it's
the component with the most duplications and its value is really the
one that gets updated).
What do you think? Please let me know if i'm heading in the wrong direction.
btw, i'll discuss why it's possible to extend the concept of zone from
the select component
to all AbstractFields in a forthcoming email (but it basically has to
do with reusing the
AbstractField#processSubmission() method)
--
Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
Apache Tapestry PMC / http://chesstu.be owner
Open Source / JEE Consulting
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: Radio and RadioGroup as AbstractField subclasses
Posted by Andreas Andreou <an...@gmail.com>.
I personnaly consider the way zone works right now in Select a hack.
And btw, RadioGroup really works with that code!
On Wed, Aug 24, 2011 at 14:26, Denis Stepanov <de...@gmail.com> wrote:
> I don't really like hacks like extracting the value or calling process submission also RadioGroup will not work because it doesn't have a client id.
>
> Right now I would rather see small changes to add support for a zone param on Checkbox and RadioGroup and refactoring after 5.3.
>
> Denis
>
> Aug 24, 2011 v 10:32 AM, Andreas Andreou:
>
>> I'm away on vacations so i didn't commit any code to apache svn.
>>
>> But you can take a look of what's involved at
>> https://github.com/andyhot/tapestry5/commits/trunk
>>
>> On Tue, Aug 23, 2011 at 13:44, Denis Stepanov <de...@gmail.com> wrote:
>>> I would like to have a zone param support for Chechbox and RadioGroup in 5.3.
>>>
>>> Will you manage to finish the refactoring into this version?
>>>
>>> Denis
>>>
>>> Aug 16, 2011 v 11:41 PM, Andreas Andreou:
>>>
>>>> :) Your patch is ok - it's basically a copy of the Select code that
>>>> did the same thing - but
>>>> what i'd like to do is investigate how to further improve on that. One
>>>> way is by generalizing
>>>> and reusing (instead of copy&pasting) and the other relates to reusing
>>>> AbstractField#processSubmission (that's what really allows impementing
>>>> this without
>>>> touching subclasses)
>>>
>>>
>>
>>
>>
>> --
>> Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
>> Apache Tapestry PMC / http://chesstu.be owner
>> Open Source / JEE Consulting
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
>> For additional commands, e-mail: dev-help@tapestry.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>
--
Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
Apache Tapestry PMC / http://chesstu.be owner
Open Source / JEE Consulting
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: Radio and RadioGroup as AbstractField subclasses
Posted by Denis Stepanov <de...@gmail.com>.
I don't really like hacks like extracting the value or calling process submission also RadioGroup will not work because it doesn't have a client id.
Right now I would rather see small changes to add support for a zone param on Checkbox and RadioGroup and refactoring after 5.3.
Denis
Aug 24, 2011 v 10:32 AM, Andreas Andreou:
> I'm away on vacations so i didn't commit any code to apache svn.
>
> But you can take a look of what's involved at
> https://github.com/andyhot/tapestry5/commits/trunk
>
> On Tue, Aug 23, 2011 at 13:44, Denis Stepanov <de...@gmail.com> wrote:
>> I would like to have a zone param support for Chechbox and RadioGroup in 5.3.
>>
>> Will you manage to finish the refactoring into this version?
>>
>> Denis
>>
>> Aug 16, 2011 v 11:41 PM, Andreas Andreou:
>>
>>> :) Your patch is ok - it's basically a copy of the Select code that
>>> did the same thing - but
>>> what i'd like to do is investigate how to further improve on that. One
>>> way is by generalizing
>>> and reusing (instead of copy&pasting) and the other relates to reusing
>>> AbstractField#processSubmission (that's what really allows impementing
>>> this without
>>> touching subclasses)
>>
>>
>
>
>
> --
> Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
> Apache Tapestry PMC / http://chesstu.be owner
> Open Source / JEE Consulting
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: Radio and RadioGroup as AbstractField subclasses
Posted by Andreas Andreou <an...@gmail.com>.
I'm away on vacations so i didn't commit any code to apache svn.
But you can take a look of what's involved at
https://github.com/andyhot/tapestry5/commits/trunk
On Tue, Aug 23, 2011 at 13:44, Denis Stepanov <de...@gmail.com> wrote:
> I would like to have a zone param support for Chechbox and RadioGroup in 5.3.
>
> Will you manage to finish the refactoring into this version?
>
> Denis
>
> Aug 16, 2011 v 11:41 PM, Andreas Andreou:
>
>> :) Your patch is ok - it's basically a copy of the Select code that
>> did the same thing - but
>> what i'd like to do is investigate how to further improve on that. One
>> way is by generalizing
>> and reusing (instead of copy&pasting) and the other relates to reusing
>> AbstractField#processSubmission (that's what really allows impementing
>> this without
>> touching subclasses)
>
>
--
Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
Apache Tapestry PMC / http://chesstu.be owner
Open Source / JEE Consulting
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: Radio and RadioGroup as AbstractField subclasses
Posted by Denis Stepanov <de...@gmail.com>.
I would like to have a zone param support for Chechbox and RadioGroup in 5.3.
Will you manage to finish the refactoring into this version?
Denis
Aug 16, 2011 v 11:41 PM, Andreas Andreou:
> :) Your patch is ok - it's basically a copy of the Select code that
> did the same thing - but
> what i'd like to do is investigate how to further improve on that. One
> way is by generalizing
> and reusing (instead of copy&pasting) and the other relates to reusing
> AbstractField#processSubmission (that's what really allows impementing
> this without
> touching subclasses)
Re: Radio and RadioGroup as AbstractField subclasses
Posted by Andreas Andreou <an...@gmail.com>.
Inline - but keep in mind what i would really like to discuss here is
if the radio
components should extend from AbstractField. The details for supporting zone
parameter in AbstractFields should be discussed in separate thread.
On Wed, Aug 17, 2011 at 00:17, Denis Stepanov <de...@gmail.com> wrote:
> Not all form fields support zone update,
Out of scope, but i hasten to add that i've already got that to work
>TextField doesn't
What do you mean? Why not update a zone on change? Or on key press?
> and Radio shouldn't too,
I mentioned about Radios, that's why i'm ok with them not being
AbstractFields. But in either case [see next]
> all radios will be binded by the RadioGroup and because of that it can't use an abstract JS init.
If the client code is smart enough, this doesn't really matter - it'll
be handled for you.
>
> By adding the zone parameter to the AbstractField you are forcing all subclasses to implement it and in some cases it just doesn't makes sense.
No subclass needs to change, all work happens inside AbstractField
>
> Just commit my patch and I will then add support for the RadioGroup.
:) Your patch is ok - it's basically a copy of the Select code that
did the same thing - but
what i'd like to do is investigate how to further improve on that. One
way is by generalizing
and reusing (instead of copy&pasting) and the other relates to reusing
AbstractField#processSubmission (that's what really allows impementing
this without
touching subclasses)
>
> Denis
>
--
Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
Apache Tapestry PMC / http://chesstu.be owner
Open Source / JEE Consulting
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: Radio and RadioGroup as AbstractField subclasses
Posted by Denis Stepanov <de...@gmail.com>.
> So, anyway, i'd like to provide the zone parameter to all
> AbstractField subclasses
> since i don't see the need to special-case it to a particular component
> and in fact i've gotten it to work quite nicely.
I don't think it's a good idea.
Not all form fields support zone update, TextField doesn't and Radio shouldn't too, all radios will be binded by the RadioGroup and because of that it can't use an abstract JS init.
By adding the zone parameter to the AbstractField you are forcing all subclasses to implement it and in some cases it just doesn't makes sense.
Just commit my patch and I will then add support for the RadioGroup.
Denis
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org