You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2015/10/29 08:31:43 UTC
Re: wicket git commit: WICKET-6013 add css classes to
AjaxFallbackOrderByBorder too
Hi Sven,
On Wed, Oct 28, 2015 at 10:02 PM, <sv...@apache.org> wrote:
> Repository: wicket
> Updated Branches:
> refs/heads/wicket-7.x 5a5ae6d73 -> e74fdcf21
>
>
> WICKET-6013 add css classes to AjaxFallbackOrderByBorder too
>
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e74fdcf2
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e74fdcf2
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e74fdcf2
>
> Branch: refs/heads/wicket-7.x
> Commit: e74fdcf219938fe3b262fdec742a7bc15694c4b6
> Parents: 5a5ae6d
> Author: Sven Meier <sv...@apache.org>
> Authored: Wed Oct 28 20:59:28 2015 +0100
> Committer: Sven Meier <sv...@apache.org>
> Committed: Wed Oct 28 20:59:28 2015 +0100
>
> ----------------------------------------------------------------------
> .../data/sort/AjaxFallbackOrderByBorder.java | 21 ++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/e74fdcf2/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
> b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
> index 7355b23..f38e53c 100644
> ---
> a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
> +++
> b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
> @@ -20,7 +20,7 @@ import org.apache.wicket.ajax.AjaxRequestTarget;
> import org.apache.wicket.ajax.attributes.IAjaxCallListener;
> import
> org.apache.wicket.extensions.markup.html.repeater.data.sort.ISortStateLocator;
> import
> org.apache.wicket.extensions.markup.html.repeater.data.sort.OrderByBorder;
> -import org.apache.wicket.markup.html.border.Border;
> +import
> org.apache.wicket.extensions.markup.html.repeater.data.sort.OrderByLink;
>
>
> /**
> @@ -35,9 +35,10 @@ import org.apache.wicket.markup.html.border.Border;
> * @author Igor Vaynberg (ivaynberg)
> *
> */
> -public abstract class AjaxFallbackOrderByBorder<S> extends Border
> +public abstract class AjaxFallbackOrderByBorder<S> extends
> OrderByBorder<S>
> {
> private static final long serialVersionUID = 1L;
> + private IAjaxCallListener ajaxCallListener;
>
IAjaxCallListener is not Serializable.
AjaxCallListener happens to be Serializable because it inherits
IComponentAwareHeaderContributor, but it is not guaranteed that the
application will use AjaxCallListener. It may use custom IACL.
>
> /**
> * Constructor
> @@ -63,9 +64,16 @@ public abstract class AjaxFallbackOrderByBorder<S>
> extends Border
> public AjaxFallbackOrderByBorder(final String id, final S
> sortProperty,
> final ISortStateLocator<S> stateLocator, final
> IAjaxCallListener ajaxCallListener)
> {
> - super(id);
> - AjaxFallbackOrderByLink<S> link = new
> AjaxFallbackOrderByLink<S>("orderByLink", sortProperty,
> - stateLocator, ajaxCallListener)
> + super(id, sortProperty, stateLocator);
> +
> + this.ajaxCallListener = ajaxCallListener;
> + }
> +
> + @Override
> + protected OrderByLink<S> newOrderByLink(String id, S property,
> + ISortStateLocator<S> stateLocator)
> + {
> + return new AjaxFallbackOrderByLink<S>("orderByLink",
> property, stateLocator, ajaxCallListener)
>
I see that AjaxFallbackOrderByLink uses this IACL field since a long time.
And it was me who migrated this from IAjaxCallDecorator (Wicket 1.5.x) to
IAjaxCallListener (Wicket 6.x)...
IMO this is bad design. Ajax** components should provide "protected void
updateAjaxAttributes(attrs)", e.g.
org.apache.wicket.ajax.markup.html.AjaxLink#updateAjaxAttributes().
This way the application developer has more control than just being able to
provide custom IACL.
I think we should improve this in 7.x and remove the IACL field in 8.x.
WDYT ?
> {
>
> private static final long serialVersionUID = 1L;
> @@ -83,10 +91,7 @@ public abstract class AjaxFallbackOrderByBorder<S>
> extends Border
>
> }
> };
> - addToBorder(link);
> - link.add(getBodyContainer());
> }
> -
> /**
> * This method is a hook for subclasses to perform an action after
> sort has changed
> */
>
>
Re: wicket git commit: WICKET-6013 add css classes to
AjaxFallbackOrderByBorder too
Posted by Martijn Dashorst <ma...@gmail.com>.
Consistency ftw!
Martijn
On Thu, Oct 29, 2015 at 8:31 AM, Martin Grigorov <mg...@apache.org> wrote:
> Hi Sven,
>
> On Wed, Oct 28, 2015 at 10:02 PM, <sv...@apache.org> wrote:
>
>> Repository: wicket
>> Updated Branches:
>> refs/heads/wicket-7.x 5a5ae6d73 -> e74fdcf21
>>
>>
>> WICKET-6013 add css classes to AjaxFallbackOrderByBorder too
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e74fdcf2
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e74fdcf2
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e74fdcf2
>>
>> Branch: refs/heads/wicket-7.x
>> Commit: e74fdcf219938fe3b262fdec742a7bc15694c4b6
>> Parents: 5a5ae6d
>> Author: Sven Meier <sv...@apache.org>
>> Authored: Wed Oct 28 20:59:28 2015 +0100
>> Committer: Sven Meier <sv...@apache.org>
>> Committed: Wed Oct 28 20:59:28 2015 +0100
>>
>> ----------------------------------------------------------------------
>> .../data/sort/AjaxFallbackOrderByBorder.java | 21 ++++++++++++--------
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/e74fdcf2/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
>> b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
>> index 7355b23..f38e53c 100644
>> ---
>> a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
>> +++
>> b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/repeater/data/sort/AjaxFallbackOrderByBorder.java
>> @@ -20,7 +20,7 @@ import org.apache.wicket.ajax.AjaxRequestTarget;
>> import org.apache.wicket.ajax.attributes.IAjaxCallListener;
>> import
>> org.apache.wicket.extensions.markup.html.repeater.data.sort.ISortStateLocator;
>> import
>> org.apache.wicket.extensions.markup.html.repeater.data.sort.OrderByBorder;
>> -import org.apache.wicket.markup.html.border.Border;
>> +import
>> org.apache.wicket.extensions.markup.html.repeater.data.sort.OrderByLink;
>>
>>
>> /**
>> @@ -35,9 +35,10 @@ import org.apache.wicket.markup.html.border.Border;
>> * @author Igor Vaynberg (ivaynberg)
>> *
>> */
>> -public abstract class AjaxFallbackOrderByBorder<S> extends Border
>> +public abstract class AjaxFallbackOrderByBorder<S> extends
>> OrderByBorder<S>
>> {
>> private static final long serialVersionUID = 1L;
>> + private IAjaxCallListener ajaxCallListener;
>>
>
> IAjaxCallListener is not Serializable.
> AjaxCallListener happens to be Serializable because it inherits
> IComponentAwareHeaderContributor, but it is not guaranteed that the
> application will use AjaxCallListener. It may use custom IACL.
>
>
>>
>> /**
>> * Constructor
>> @@ -63,9 +64,16 @@ public abstract class AjaxFallbackOrderByBorder<S>
>> extends Border
>> public AjaxFallbackOrderByBorder(final String id, final S
>> sortProperty,
>> final ISortStateLocator<S> stateLocator, final
>> IAjaxCallListener ajaxCallListener)
>> {
>> - super(id);
>> - AjaxFallbackOrderByLink<S> link = new
>> AjaxFallbackOrderByLink<S>("orderByLink", sortProperty,
>> - stateLocator, ajaxCallListener)
>> + super(id, sortProperty, stateLocator);
>> +
>> + this.ajaxCallListener = ajaxCallListener;
>> + }
>> +
>> + @Override
>> + protected OrderByLink<S> newOrderByLink(String id, S property,
>> + ISortStateLocator<S> stateLocator)
>> + {
>> + return new AjaxFallbackOrderByLink<S>("orderByLink",
>> property, stateLocator, ajaxCallListener)
>>
>
> I see that AjaxFallbackOrderByLink uses this IACL field since a long time.
> And it was me who migrated this from IAjaxCallDecorator (Wicket 1.5.x) to
> IAjaxCallListener (Wicket 6.x)...
> IMO this is bad design. Ajax** components should provide "protected void
> updateAjaxAttributes(attrs)", e.g.
> org.apache.wicket.ajax.markup.html.AjaxLink#updateAjaxAttributes().
> This way the application developer has more control than just being able to
> provide custom IACL.
> I think we should improve this in 7.x and remove the IACL field in 8.x.
> WDYT ?
>
>
>> {
>>
>> private static final long serialVersionUID = 1L;
>> @@ -83,10 +91,7 @@ public abstract class AjaxFallbackOrderByBorder<S>
>> extends Border
>>
>> }
>> };
>> - addToBorder(link);
>> - link.add(getBodyContainer());
>> }
>> -
>> /**
>> * This method is a hook for subclasses to perform an action after
>> sort has changed
>> */
>>
>>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com