currently we only invoke configure before the render. this would mean we would have to invoke it before processing a listener, clearing the cache, and then invoking it again before render. i wonder if that is enough places to invoke it....
-igor On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <[email protected]> wrote: > If user click an link, it will change the value of some property at the > process_event request cycle step. Then the processor will go to the respond > step, will invoke every component before render method which will end up > invoking the Component#configure and updating the visibility/enabled state > (even if it changes, we are able to work with the updated state). So when > the this component has the opportunity to render it self, it will be aware > its update state. > > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <[email protected]>wrote: > >> there are other places that should be checked though. for example >> before we invoke a listener on the component we should check again to >> make sure that visibility hasnt changed. eg if visibility depends on >> some property of the user clicking the link that changed between >> render and clicking the link. >> >> -igor >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <[email protected]> wrote: >> > An implementation idea: >> > >> > Component { >> > public final void configure() >> > { >> > if (!getFlag(FLAG_CONFIGURED)) >> > { >> > setVisible_NoClientCode(isVisible()); //we only check the user >> > isVisible in here >> > onConfigure(); >> > setFlag(FLAG_CONFIGURED, true); >> > } >> > } >> > } >> > >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <[email protected] >> >wrote: >> > >> >> so how is it different if they can still override something that needs >> >> to be checked all the time? >> >> >> >> -igor >> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <[email protected]> >> wrote: >> >> > I understand the concern about possible isVisible implementations like >> >> > >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this component >> >> being >> >> > rendered at 09:59:59 >> >> > isVisible(return dao.list().size() > 0);// performance issues >> >> > >> >> > But maybe we can have the best from both approaches. This is an >> >> copy/paste >> >> > from java.awt.Component: >> >> > >> >> > public boolean isVisible() { >> >> > return isVisible_NoClientCode(); >> >> > } >> >> > final boolean isVisible_NoClientCode() { >> >> > return visible; >> >> > } >> >> > >> >> > There are some points in the awt framework were the isVisible method >> is >> >> not >> >> > used in benefit of isVisible_NoClientCode >> >> > I'm in favor of create an final isVisible/Enabled version and change >> the >> >> > Wicket core to use it. Also maintain the hotspot to users provide >> their >> >> > isVisible/Enable implementations that will serve to feed the core >> >> component >> >> > state. >> >> > >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg < >> [email protected] >> >> >wrote: >> >> > >> >> >> ive run into plenty of weird problems with overrides, but maybe >> >> >> because this was in a high concurrency app where data changed >> >> >> frequently. the problems arise from the fact that the value returned >> >> >> from isvisible() can change while we are doing traversals, etc. >> >> >> >> >> >> eg we run a traversal for all visible components and put them in a >> >> >> list. later we iterate over the list and try to render these >> >> >> components. the render function also checks their visibility and if >> >> >> they are no longer visible it throws an exception. >> >> >> >> >> >> if isvisible() override depends on some external factor like the >> >> >> database there is a small window where the value can change and now >> >> >> you can have a weird exception: such as "tried to invoke a listener >> on >> >> >> a component that is not visible or not enabled". these are very >> >> >> intermittent and damn near impossible to reproduce. >> >> >> >> >> >> another problem is performance. isvisible() is called multiple times >> >> >> during the request and if it depends on the database it can be a >> >> >> performance problem. in fact a couple of users have complained about >> >> >> this on the list in the past. at least now we have an easy solution >> >> >> for them - use onconfigure(). >> >> >> >> >> >> so as of right now the developers have two choices: override >> >> >> isvisible() and potentially suffer the consequences. or, override >> >> >> onconfigure() and set visibility there in a more deterministic >> >> >> fashion. >> >> >> >> >> >> -igor >> >> >> >> >> >> >> >> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius >> >> >> <[email protected]> wrote: >> >> >> > To expand, unless I'm missing something (new?), things are really >> only >> >> >> > problematic when both the mutable value and the override are mixed. >> In >> >> >> > a way, I think that using the override is 'more pure', as it's a >> >> >> > simple function that is executed when needed, whereas mutable state >> >> >> > can be harder to deal with when trying to figure out how it got to >> be >> >> >> > in that state. So, sorry Igor, but we disagree on this one. >> >> >> > >> >> >> > Eelco >> >> >> > >> >> >> > >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius >> >> >> > <[email protected]> wrote: >> >> >> >> Niether is evil. It has potential pitfalls, which you should just >> be >> >> >> >> aware of. We use such overrides all over the place and never have >> >> >> >> problems with them either. :-) Avoiding it is safer, but also more >> >> >> >> verbose (in 1.3.x at least). >> >> >> >> >> >> >> >> Eelco >> >> >> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg < >> >> [email protected]> >> >> >> wrote: >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <[email protected]> >> >> wrote: >> >> >> >>>> Hi Douglas, >> >> >> >>>> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility of a >> >> >> >>>> component changes while its form is being processed. >> >> >> >>>> In our projects we're overriding isVisible() where appropriate >> and >> >> >> never >> >> >> >>>> encountered a similar problem. >> >> >> >>>> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is >> >> overriding >> >> >> >>>> isEnabled() going to be declared evil too? ;) >> >> >> >>> >> >> >> >>> yes >> >> >> >>> >> >> >> >>> -igor >> >> >> >>> >> >> >> >>>> >> >> >> >>>> Sven >> >> >> >>>> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote: >> >> >> >>>> >> >> >> >>>>> Can you explain why? We have done this all over the place. >> >> >> >>>>> >> >> >> >>>>> D/ >> >> >> >>>>> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote: >> >> >> >>>>> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to override >> >> >> onConfigure() >> >> >> >>>>> > and call setVisible(true|false) depending on your conditions. >> >> >> >>>>> > >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson < >> >> >> >>>>> > [email protected]> wrote: >> >> >> >>>>> > >> >> >> >>>>> >> Igor posted a comment to this bug saying that overriding >> >> >> isVisible() is >> >> >> >>>>> >> "evil" >> >> >> >>>>> >> >> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171 >> >> >> >>>>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear more. >> >> >> >>>>> >> >> >> >> >>>>> >> D/ >> >> >> >>>>> >> >> >> >>>> >> >> >> >>>> >> >> >> >>>> >> >> >> >>> >> >> >> >> >> >> >> > >> >> >> >> >> > >> >> > >> >> > >> >> > -- >> >> > Pedro Henrique Oliveira dos Santos >> >> > >> >> >> > >> > >> > >> > -- >> > Pedro Henrique Oliveira dos Santos >> > >> > > > > -- > Pedro Henrique Oliveira dos Santos >
