wicket-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pedro Santos <pedros...@gmail.com>
Subject Re: disabled components/behaviors fixes of WICKET-3098
Date Wed, 24 Nov 2010 16:21:11 GMT
Why don't simple change the AbstractBehavior default isEnabled
implementation to respect the component state, like:

    public boolean isEnabled(Component component)
    {
        return component.isEnabled();
    }

It would solve the problem at the ticket. IMO the proposed interface is
prolix because the behavior enabled/disabled state is enough information to
decide to silent return or not an ajax call to it.

On Wed, Nov 24, 2010 at 2:12 PM, Johan Compagner <jcompagner@gmail.com>wrote:

> yes that i did see in 1.5
> but in 1.4 i guess the only thing we can do is just introduce that
> IIgnoreDisabledComponentBehavior interface (that doesnt have any
> method)
>
> (or something the same with a different name, any recomendations?)
>
> johan
>
>
> On Wed, Nov 24, 2010 at 16:57, Igor Vaynberg <igor.vaynberg@gmail.com>
> wrote:
> > i added boolean canCallListenerInterface(Component component) to
> ibehavior
> >
> > -igor
> >
> > On Wed, Nov 24, 2010 at 12:54 AM, Johan Compagner <jcompagner@gmail.com>
> wrote:
> >> which method name?
> >>
> >> I currently only have a tagging interface that tags this behavior...
> >>
> >> On Wed, Nov 24, 2010 at 03:00, Igor Vaynberg <igor.vaynberg@gmail.com>
> wrote:
> >>> i just fixed it in trunk. in 1.4 we dont really have a choice but to
> >>> add the mixin interface, but can you at least make the method name
> >>> match?
> >>>
> >>> cheers,
> >>> -igor
> >>>
> >>> On Tue, Nov 23, 2010 at 1:04 PM, Johan Compagner <jcompagner@gmail.com>
> wrote:
> >>>> that i find a big hack
> >>>> then i need to have some none visible (in html not in java) label or
> >>>> something.. thats horrible then i need to add all kinds of stuff.
> >>>>
> >>>> I just want that behavior is executed. by a call. that call is not a
> >>>> user event (at least not directly)
> >>>> it is just (in this example) a resize event that is unrelated to the
> >>>> component being disabled. Disabled is just the look in the ui it
> >>>> doesnt mean that resizing shouldnt happen
> >>>>
> >>>> I get that disabled behaviors shouldnt work, i also get that behaviors
> >>>> shouldnt work on none visible components (there is no ui)
> >>>> but disabled components is just a visual thing. that doesnt mean that
> >>>> really all events are also disabled.
> >>>>
> >>>> for example in Swing on a button yes the click event (ActionListenter)
> >>>> is disabled but a ComponentListener (add/remove) still will work
> >>>> fine..
> >>>>
> >>>> johan
> >>>>
> >>>>
> >>>> On Tue, Nov 23, 2010 at 21:52, Martin Grigorov <mgrigorov@apache.org>
> wrote:
> >>>>> On Tue, Nov 23, 2010 at 11:02 AM, Johan Compagner <
> jcompagner@gmail.com>wrote:
> >>>>>
> >>>>>> i am currently using this patch for our wicket code so that
we can
> move
> >>>>>> on..
> >>>>>>
> >>>>>>
> >>>>>> First check is that if it is an ajax request for that behavior
then
> >>>>>> just throw an abort exception..
> >>>>>> Dont just return null because the constantly a full page render
is
> >>>>>> done instead of the ajax behavior request.
> >>>>>>
> >>>>>> Second is that i introduced an tagging interface so that behaviors
> can
> >>>>>> make them self work for disabled components if they want to.
> >>>>>>
> >>>>>> anybody a better idea?
> >>>>>>
> >>>>> What about adding a companion component for the resize logic ? I.e.
> the
> >>>>> ListView will be disabled, but the companion component will fire
and
> repaint
> >>>>> the ListView if necessary. This way you explicitly say that you
want
> to
> >>>>> modify disabled component.
> >>>>>
> >>>>>>
> >>>>>> johan
> >>>>>>
> >>>>>> ### Eclipse Workspace Patch 1.0
> >>>>>> #P wicket
> >>>>>> Index:
> >>>>>>
> src/main/java/org/apache/wicket/behavior/IIgnoreDisabledComponentBehavior.java
> >>>>>> ===================================================================
> >>>>>> ---
> >>>>>>
> src/main/java/org/apache/wicket/behavior/IIgnoreDisabledComponentBehavior.java
> >>>>>>      (revision
> >>>>>> 0)
> >>>>>> +++
> >>>>>>
> src/main/java/org/apache/wicket/behavior/IIgnoreDisabledComponentBehavior.java
> >>>>>>      (revision
> >>>>>> 0)
> >>>>>> @@ -0,0 +1,28 @@
> >>>>>> +/*
> >>>>>> + * Licensed to the Apache Software Foundation (ASF) under one
or
> more
> >>>>>> + * contributor license agreements.  See the NOTICE file distributed
> with
> >>>>>> + * this work for additional information regarding copyright
> ownership.
> >>>>>> + * The ASF licenses this file to You under the Apache License,
> Version 2.0
> >>>>>> + * (the "License"); you may not use this file except in compliance
> with
> >>>>>> + * the License.  You may obtain a copy of the License at
> >>>>>> + *
> >>>>>> + *      http://www.apache.org/licenses/LICENSE-2.0
> >>>>>> + *
> >>>>>> + * Unless required by applicable law or agreed to in writing,
> software
> >>>>>> + * distributed under the License is distributed on an "AS IS"
> BASIS,
> >>>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
or
> >>>>>> implied.
> >>>>>> + * See the License for the specific language governing permissions
> and
> >>>>>> + * limitations under the License.
> >>>>>> + */
> >>>>>> +package org.apache.wicket.behavior;
> >>>>>> +
> >>>>>> +/**
> >>>>>> + * Interface that can be used to tag behaviors where response
> should
> >>>>>> be called on even if the
> >>>>>> + * component is disabled.
> >>>>>> + *
> >>>>>> + * @author jcompagner
> >>>>>> + */
> >>>>>> +public interface IIgnoreDisabledComponentBehavior extends IBehavior
> >>>>>> +{
> >>>>>> +
> >>>>>> +}
> >>>>>> Index:
> >>>>>>
> src/main/java/org/apache/wicket/request/target/component/listener/BehaviorRequestTarget.java
> >>>>>> ===================================================================
> >>>>>> ---
> >>>>>>
> src/main/java/org/apache/wicket/request/target/component/listener/BehaviorRequestTarget.java
> >>>>>>        (revision
> >>>>>> 1033481)
> >>>>>> +++
> >>>>>>
> src/main/java/org/apache/wicket/request/target/component/listener/BehaviorRequestTarget.java
> >>>>>>        (working
> >>>>>> copy)
> >>>>>> @@ -18,13 +18,16 @@
> >>>>>>
> >>>>>>  import java.util.List;
> >>>>>>
> >>>>>> +import org.apache.wicket.AbortException;
> >>>>>>  import org.apache.wicket.Component;
> >>>>>>  import org.apache.wicket.Page;
> >>>>>>  import org.apache.wicket.RequestCycle;
> >>>>>>  import org.apache.wicket.RequestListenerInterface;
> >>>>>>  import org.apache.wicket.behavior.IBehavior;
> >>>>>>  import org.apache.wicket.behavior.IBehaviorListener;
> >>>>>> +import org.apache.wicket.behavior.IIgnoreDisabledComponentBehavior;
> >>>>>>  import org.apache.wicket.protocol.http.PageExpiredException;
> >>>>>> +import org.apache.wicket.protocol.http.WebRequest;
> >>>>>>  import org.apache.wicket.request.RequestParameters;
> >>>>>>  import org.slf4j.Logger;
> >>>>>>  import org.slf4j.LoggerFactory;
> >>>>>> @@ -85,13 +88,6 @@
> >>>>>>                // Get the IBehavior for the component based
on the
> request
> >>>>>> parameters
> >>>>>>                final Component component = getTarget();
> >>>>>>
> >>>>>> -               if (!component.isVisibleInHierarchy() ||
> >>>>>> !component.isEnabledInHierarchy())
> >>>>>> -               {
> >>>>>> -                       // ignore this request
> >>>>>> -                       logger.warn("component not enabled or
> visible;
> >>>>>> ignoring call.
> >>>>>> Component: {}", component);
> >>>>>> -                       return;
> >>>>>> -               }
> >>>>>> -
> >>>>>>                final String id =
> getRequestParameters().getBehaviorId();
> >>>>>>                if (id == null)
> >>>>>>                {
> >>>>>> @@ -124,6 +120,11 @@
> >>>>>>                                        logger.warn(
> >>>>>>                                                "behavior not
> enabled;
> >>>>>> ignoring call. behavior: {} at index: {}
> >>>>>> on component: {}",
> >>>>>>                                                new Object[]
{
> behavior,
> >>>>>> idAsInt, component });
> >>>>>> +                                       if
> (requestCycle.getRequest()
> >>>>>> instanceof WebRequest &&
> >>>>>> +
> >>>>>> ((WebRequest)requestCycle.getRequest()).isAjax())
> >>>>>> +                                       {
> >>>>>> +                                               throw new
> AbortException();
> >>>>>> +                                       }
> >>>>>>                                        return;
> >>>>>>                                }
> >>>>>>
> >>>>>> @@ -138,6 +139,20 @@
> >>>>>>                                "; Component: " +
> component.toString());
> >>>>>>                }
> >>>>>>
> >>>>>> +               if (!component.isVisibleInHierarchy() ||
> >>>>>> +                       (!(behaviorListener instanceof
> >>>>>> IIgnoreDisabledComponentBehavior)
> >>>>>> && !component.isEnabledInHierarchy()))
> >>>>>> +               {
> >>>>>> +                       // ignore this request
> >>>>>> +                       logger.warn("component not enabled or
> visible;
> >>>>>> ignoring call.
> >>>>>> Component: {}", component);
> >>>>>> +                       if (requestCycle.getRequest() instanceof
> WebRequest
> >>>>>> &&
> >>>>>> +
> >>>>>> ((WebRequest)requestCycle.getRequest()).isAjax())
> >>>>>> +                       {
> >>>>>> +                               throw new AbortException();
> >>>>>> +                       }
> >>>>>> +                       return;
> >>>>>> +               }
> >>>>>> +
> >>>>>> +
> >>>>>>                // Invoke the interface method
> >>>>>>                behaviorListener.onRequest();
> >>>>>>         }
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Tue, Nov 23, 2010 at 09:42, Johan Compagner <
> jcompagner@gmail.com>
> >>>>>> wrote:
> >>>>>> > hi,
> >>>>>> >
> >>>>>> > i suddenly bump big time into this issue that is fixed.
> >>>>>> > ( https://issues.apache.org/jira/browse/WICKET-3098 )
> >>>>>> >
> >>>>>> > I get that disabled behaviors can't be used to do respond,
because
> >>>>>> > that behavior shouldnt be rendered in the first place.
> >>>>>> >
> >>>>>> > But the fix also goes one step deeper... It also blocks
if the
> >>>>>> > component is disabled.. That is a big problem
> >>>>>> >
> >>>>>> > Because a disabled component is rendered, and all the behaviors
> are
> >>>>>> > accepted and rendered also so now suddenly a behavior is
rendered
> >>>>>> > (because the behavior is not disabled)
> >>>>>> > but a callback will fail...
> >>>>>> >
> >>>>>> > thats something i dont like...because now i get loads of
these in
> the
> >>>>>> log:
> >>>>>> >
> >>>>>> > 2010-11-23 09:10:57,934 WARN [http-8080-1]
> >>>>>> >
> org.apache.wicket.request.target.component.listener.BehaviorRequestTarget
> >>>>>> > - component not enabled or visible; ignoring call. Component:
> >>>>>> > [MarkupContainer [Component id = View]]
> >>>>>> >
> >>>>>> >
> >>>>>> > and the worse thing is if the behavior blocks like that
it fall
> backs
> >>>>>> > to a IRedirectListener so it rerenders the whole page and
that
> again
> >>>>>> > renders the disabled component with its behavior and it
starts all
> >>>>>> > over again and again and again.
> >>>>>> >
> >>>>>> > the example i have here is that we have a ListView/Repeater
with
> some
> >>>>>> > paging component and that listview has a behavior attached
that
> does a
> >>>>>> > call back when it got first rendered to give us back the
sizes it
> has
> >>>>>> > in the browser
> >>>>>> > and if we see that it has way more space then it currently
shows
> (if
> >>>>>> > it now shows 10 rows and it has space for 20) we rerender
the
> ListView
> >>>>>> > again but then with a bigger visible row count.
> >>>>>> > that is a behavior of the ListView, but the listview can
be in a
> >>>>>> > disabled state (because a user first have to press a button
of
> "edit"
> >>>>>> > or something like that) but that resizing i still want
to happen
> if if
> >>>>>> > the ListView is disabled...
> >>>>>> >
> >>>>>> > So i like to some how tell that that this behavior should
be
> called.
> >>>>>> > Now we do this:
> >>>>>> >
> >>>>>> >
> >>>>>> > test component enablement
> >>>>>> > test behavior id
> >>>>>> > test behaviors
> >>>>>> > test behaviors enabledment.
> >>>>>> > call behavior
> >>>>>> >
> >>>>>> > i like to turn that around
> >>>>>> >
> >>>>>> > test behavior id
> >>>>>> > test behaviors
> >>>>>> > test behaviors enabledment.
> >>>>>> > test component enablement IF behavior doesnt implement
> >>>>>> IWorkForDisabledComponent
> >>>>>> > call behavior
> >>>>>> >
> >>>>>> > So that a developer can be explicit in that check..
> >>>>>> >
> >>>>>> > We could also introduce a public method on Component:
> >>>>>> > isEnabledFor(IBehavior) which returns defaults its own
enable
> state.
> >>>>>> >
> >>>>>> > johan
> >>>>>> >
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>



-- 
Pedro Henrique Oliveira dos Santos

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message