i've seen quite a lot of places (like in Page, for example) where there's
code cut and
pasted like in MarkupContainer.internalAttach2()
we should probably have visitChildrenAndSelf to avoid this sort of thing.
Johan Compagner wrote:
>
> i think we have it covered now
> because we are setting the flag we test for up front (on all the
> components
> of the page)
> then we call onAttach on all of them.
> So the flag (there are now many flags) is now again seens as one thing
> because it is set
> in a single call and reset in a single call. (call == visitor)
>
> the problem is with a request cycle flag (which we could still do) that we
> also have partial attach calls
> for example auto components or components that are new and added to a
> markup
> container of an already attached page
> So page is not always the common entry point, but any kind of component
> could be
>
>
> johan
>
>
>
> On 5/15/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
>>
>>
>>
>> But neither order is going to work. The problem is that the check should
>> not be on any component state, but on a global phase, probably held in
>> the
>> request cycle (which already know what phase that is, somewhere in
>> there).
>> I personally think you're right that a component is not attached until it
>> has attached its children. And we should definitely leave the component
>> state changes I made in place. We just need to do that check based on
>> something outside the component hierarchy. Since Page is not a reliable
>> place to put that because components may not have a Page yet, it seems
>> like
>> RequestCycle would be the right place. After all, this IS the phase of
>> the
>> request cycle.
>>
>>
>> Jonathan Locke wrote:
>> >
>> >
>> > actually, this is how i had it before almaw told me that would break
>> > everything.
>> >
>> >
>> > Johan Compagner wrote:
>> >>
>> >> ok we really need to fix this because this is broken now
>> >>
>> >> what i can do is in attach() this:
>> >>
>> >> setFlag(FLAG_ATTACHED, true);
>> >> attachChildren();
>> >>
>> >> swap:
>> >> attachChildren();
>> >> setFlag(FLAG_ATTACHED, true);
>> >>
>> >> (i think we should do this anyway)
>> >>
>> >> and then in checkForHierarchy()
>> >>
>> >> if (!isAttached())
>> >> {
>> >> throw new WicketRuntimeException(
>> >> "Cannot modify component unattached components");
>> >> }
>> >>
>> >> so test for is it attached()
>> >> that sounds logical
>> >>
>> >> except one problem in the constructor currently the component s not
>> >> attached
>> >> so what i can do is make a component when it is created default
>> attached
>> >> (that sounds also logical to me)
>> >> but then onAttach() is the first time not called (because that was the
>> >> constructor)
>> >>
>> >> If this is not a good idea we need another flag or something (on page)
>> >> where
>> >> we test really if the page
>> >> is in the right state. Because the whole attach cycle nothing should
>> be
>> >> possible to change hierachy.
>> >>
>> >> johan
>> >>
>> >> On 5/14/07, Johan Compagner <[EMAIL PROTECTED]> wrote:
>> >>>
>> >>> hmm also now we don't test globally any more things can go wrong
>> >>>
>> >>> a component can't alter itself. But it can alter everything else!
>> >>>
>> >>> as i said it can do: parent.add/remove
>> >>>
>> >>> but it can also do: child.add /remove (because that one isn't in
>> >>> attaching
>> >>> yet...)
>> >>>
>> >>>
>> >>>
>> >>> On 5/14/07, Johan Compagner <[EMAIL PROTECTED] > wrote:
>> >>> >
>> >>> > the same goes i guess for attaching:
>> >>> >
>> >>> > setFlag(FLAG_ATTACHING, false);
>> >>> > setFlag(FLAG_ATTACHED, true);
>> >>> > attachChildren();
>> >>> >
>> >>> >
>> >>> > thats also wrong. The parent is attaching false but then the
>> children
>> >>> > attach is called
>> >>> > so a child can do this:
>> >>> >
>> >>> > parent.remove(this)
>> >>> >
>> >>> > that would work fine now in attaching mode as far as i can see.
>> >>> >
>> >>> > johan
>> >>> >
>> >>> >
>> >>> > On 5/14/07, Johan Compagner <[EMAIL PROTECTED]> wrote:
>> >>> > >
>> >>> > > hmm this could go wrong now:
>> >>> > >
>> >>> > > setFlag(FLAG_RENDERING, true);
>> >>> > > onBeforeRenderChildren();
>> >>> > >
>> >>> > >
>> >>> > > we first set the flag in rendering then call the onBefore on all
>> the
>> >>> > > childs
>> >>> > >
>> >>> > > so the parent is already i render mode (can't be changed) but the
>> >>> > > childs should be able to still alter the state
>> >>> > > thats a bit strange i think the rendering flag should be on true
>> >>> only
>> >>> > > if all the childs are also called? agree?
>> >>> > >
>> >>> > > johan
>> >>> > >
>> >>> > >
>> >>> > > On 5/14/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
>> >>> > > >
>> >>> > > >
>> >>> > > >
>> >>> > > > I committed a fix to this this morning that I have confidence
>> >>> > > > in. Still
>> >>> > > > having problems due to this refactor. Who did it?
>> >>> > > >
>> >>> > > >
>> >>> > > > Jonathan Locke wrote:
>> >>> > > > >
>> >>> > > > >
>> >>> > > > > unless you can get to this fast, i'm going to guess a fix
>> >>> because
>> >>> > > > > we're blocked on this.
>> >>> > > > >
>> >>> > > > >
>> >>> > > > > Johan Compagner wrote:
>> >>> > > > >>
>> >>> > > > >> will check it out asap
>> >>> > > > >>
>> >>> > > > >> On 5/12/07, Jonathan Locke < [EMAIL PROTECTED]>
>> wrote:
>> >>> > > > >>>
>> >>> > > > >>>
>> >>> > > > >>>
>> >>> > > > >>> Opened new blocking bug:
>> >>> > > > >>>
>> >>> > > > >>> https://issues.apache.org/jira/browse/WICKET-558
>> >>> > > > >>>
>> >>> > > > >>>
>> >>> > > > >>> Jonathan Locke wrote:
>> >>> > > > >>> >
>> >>> > > > >>> >
>> >>> > > > >>> > Because components are created in onBeforeRender in list
>> >>> views
>> >>> > > > (it
>> >>> > > > >>> calls
>> >>> > > > >>> > populateItem then), it is necessary for AJAX renderings
>> now
>> >>> to
>> >>> > > > call
>> >>> > > > >>> > beforeRender on every component to be updated before
>> >>> > > > proceeding with
>> >>> > > > >>> the
>> >>> > > > >>> > render.
>> >>> > > > >>> >
>> >>> > > > >>> > It looks like AjaxRequestTarget.respondComponent should
>> call
>> >>> > > > before
>> >>> > > > >>> render
>> >>> > > > >>> > on the component tree it's about to render. But I'm
>> unsure
>> >>> if
>> >>> > > > this is
>> >>> > > > >>> the
>> >>> > > > >>> > right place to make the change. Johan can you look at
>> this
>> >>> > > > since you
>> >>> > > > >>> were
>> >>> > > > >>> > doing this refactor and probably know all the details?
>> >>> > > > >>> >
>> >>> > > > >>> >
>> >>> > > > >>>
>> >>> > > > >>> --
>> >>> > > > >>> View this message in context:
>> >>> > > > >>>
>> >>> > > >
>> >>>
>> http://www.nabble.com/New-attach-%3EbeforeRender-refactor-breaks-ajax-updating-of-list-views-tf3729131.html#a10443272
>> >>> > > > >>> Sent from the Wicket - Dev mailing list archive at
>> Nabble.com
>> >>> .
>> >>> > > > >>>
>> >>> > > > >>>
>> >>> > > > >>
>> >>> > > > >>
>> >>> > > > >
>> >>> > > > >
>> >>> > > >
>> >>> > > > --
>> >>> > > > View this message in context:
>> >>> > > >
>> >>>
>> http://www.nabble.com/New-attach-%3EbeforeRender-refactor-breaks-ajax-updating-of-list-views-tf3729131.html#a10609034
>> >>> > > > Sent from the Wicket - Dev mailing list archive at Nabble.com.
>> >>> > > >
>> >>> > > >
>> >>> > >
>> >>> >
>> >>>
>> >>
>> >>
>> >
>> >
>>
>> --
>> View this message in context:
>> http://www.nabble.com/New-attach-%3EbeforeRender-refactor-breaks-ajax-updating-of-list-views-tf3729131.html#a10614119
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>>
>
>
--
View this message in context:
http://www.nabble.com/New-attach-%3EbeforeRender-refactor-breaks-ajax-updating-of-list-views-tf3729131.html#a10614262
Sent from the Wicket - Dev mailing list archive at Nabble.com.
