You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Oscar Bueno <os...@isencia.com> on 2006/12/15 15:36:02 UTC

Component reAttach and versioning

I'm reAttaching a component doing something like:

MyFooPanel p1 = new MyFooPanel(this, "panel";);
MyBarPanel p2 = new MyBarPanel(this, "panel");
p1.reAttach();

When I try to restore to the initial page version I found that the component
with id "panel" is not a children component of the page.

I have investigated it and I think it is because when the component is
reAttached the order in which the changes are added to the ChangesList is:
- Add p2.
- Remove p1.

When the initial version is restored the undo functionality is done in
reverse mode like, 
- Add p1.
- Remove p2.

The problem is p1 and p2 have the same id, so when p2 is removed what is
removing is p1 that has just added.

Oscar.
-- 
View this message in context: http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7892497
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: Component reAttach and versioning

Posted by Oscar Bueno <os...@isencia.com>.
It works now fine with the last fix.


Oscar Bueno wrote:
> 
> Now the component p1 is added, but it still have a problem with the flag
> FLAG_REMOVED_FROM_PARENT that is set to true.  It was set to true in the
> MarkupContainer add method when it was reattached, but the undo method the
> flag to false.
> 
> Oscar.
> 
> 
> 
> 
> 
> Johan Compagner wrote:
>> 
>> thx i am looking at it now.
>> 
>> How it is get undone is the right way.
>> You do exactly the reverse way of how the undo able actions are added.
>> So that is fine.
>> Then i can fix it i guess in MarkupContainer.add(Component) by calling
>> addedComponent a bit later
>> (after the if(replaces != null))
>> 
>> but i think the real problem is in this code:
>> 
>> private final int children_indexOf(Component<?> child)
>>     {
>>         if (children instanceof Component)
>>         {
>>             if (((Component)children).getId().equals(child.getId()))
>>             {
>>                 return 0;
>>             }
>>         }
>>         else
>>         {
>>             if (children != null)
>>             {
>>                 final Component[] components = (Component[])children;
>>                 for (int i = 0; i < components.length; i++)
>>                 {
>>                     if (components[i].getId().equals(child.getId()))
>>                     {
>>                         return i;
>>                     }
>>                 }
>>             }
>>         }
>>         return -1;
>>     }
>> 
>> especially the 2 ifs:
>> 
>> if (components[i].getId().equals(child.getId()))
>> 
>> That is not right in the first place. If you have this:
>> 
>> MyFooPanel p1 = new MyFooPanel(this, "panel";);
>> MyBarPanel p2 = new MyBarPanel(this, "panel");
>> 
>> 
>> then p2 is the one that is in the parent. (p1 is in standby...)
>> 
>> if you do then this:
>> 
>> this.remove(p1)
>> 
>> Then what now happens is that p2 is removed!
>> Thats wrong in my eyes. That shouldn't happen you ask for p1 to get
>> removed..
>> you don't ask for remove("panel");
>> 
>> So i will fix that that indexOf(component) will not look at the id but
>> just
>> at the identity of the component.
>> Do anybody see a problem with this?
>> 
>> should i also fix the order of which these 2 methods are called:
>> 
>> addedComponent(child);
>> removedComponent(replaced);
>> 
>> i thnk it is more logical to do it the other way:
>> 
>> removedComponent(replaced);
>> addedComponent(child);
>> 
>> because you can't add one if you don't remove one first...
>> Also here i don't think this will cause a problem.
>> 
>> johan
>> 
>> 
>> 
>> On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>>>
>>>
>>> Jira issue WICKET-172 created
>>>
>>> oscar.
>>>
>>>
>>> Johan Compagner wrote:
>>> >
>>> > please make a jira issue for this. Then i will look at it asap.
>>> >
>>> > johan
>>> >
>>> >
>>> > On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>>> >>
>>> >>
>>> >> I'm reAttaching a component doing something like:
>>> >>
>>> >> MyFooPanel p1 = new MyFooPanel(this, "panel";);
>>> >> MyBarPanel p2 = new MyBarPanel(this, "panel");
>>> >> p1.reAttach();
>>> >>
>>> >> When I try to restore to the initial page version I found that the
>>> >> component
>>> >> with id "panel" is not a children component of the page.
>>> >>
>>> >> I have investigated it and I think it is because when the component
>>> is
>>> >> reAttached the order in which the changes are added to the
>>> ChangesList
>>> >> is:
>>> >> - Add p2.
>>> >> - Remove p1.
>>> >>
>>> >> When the initial version is restored the undo functionality is done
>>> in
>>> >> reverse mode like,
>>> >> - Add p1.
>>> >> - Remove p2.
>>> >>
>>> >> The problem is p1 and p2 have the same id, so when p2 is removed what
>>> is
>>> >> removing is p1 that has just added.
>>> >>
>>> >> Oscar.
>>> >> --
>>> >> View this message in context:
>>> >>
>>> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7892497
>>> >> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>> >>
>>> >>
>>> >
>>> >
>>>
>>> --
>>> View this message in context:
>>> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7895031
>>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>>
>>>
>> 
>> 
> 
> 

-- 
View this message in context: http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7915758
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: Component reAttach and versioning

Posted by Oscar Bueno <os...@isencia.com>.
Now the component p1 is added, but it still have a problem with the flag
FLAG_REMOVED_FROM_PARENT that is set to true.  It was set to true in the
MarkupContainer add method when it was reattached, but the undo method the
flag to false.

Oscar.





Johan Compagner wrote:
> 
> thx i am looking at it now.
> 
> How it is get undone is the right way.
> You do exactly the reverse way of how the undo able actions are added.
> So that is fine.
> Then i can fix it i guess in MarkupContainer.add(Component) by calling
> addedComponent a bit later
> (after the if(replaces != null))
> 
> but i think the real problem is in this code:
> 
> private final int children_indexOf(Component<?> child)
>     {
>         if (children instanceof Component)
>         {
>             if (((Component)children).getId().equals(child.getId()))
>             {
>                 return 0;
>             }
>         }
>         else
>         {
>             if (children != null)
>             {
>                 final Component[] components = (Component[])children;
>                 for (int i = 0; i < components.length; i++)
>                 {
>                     if (components[i].getId().equals(child.getId()))
>                     {
>                         return i;
>                     }
>                 }
>             }
>         }
>         return -1;
>     }
> 
> especially the 2 ifs:
> 
> if (components[i].getId().equals(child.getId()))
> 
> That is not right in the first place. If you have this:
> 
> MyFooPanel p1 = new MyFooPanel(this, "panel";);
> MyBarPanel p2 = new MyBarPanel(this, "panel");
> 
> 
> then p2 is the one that is in the parent. (p1 is in standby...)
> 
> if you do then this:
> 
> this.remove(p1)
> 
> Then what now happens is that p2 is removed!
> Thats wrong in my eyes. That shouldn't happen you ask for p1 to get
> removed..
> you don't ask for remove("panel");
> 
> So i will fix that that indexOf(component) will not look at the id but
> just
> at the identity of the component.
> Do anybody see a problem with this?
> 
> should i also fix the order of which these 2 methods are called:
> 
> addedComponent(child);
> removedComponent(replaced);
> 
> i thnk it is more logical to do it the other way:
> 
> removedComponent(replaced);
> addedComponent(child);
> 
> because you can't add one if you don't remove one first...
> Also here i don't think this will cause a problem.
> 
> johan
> 
> 
> 
> On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>>
>>
>> Jira issue WICKET-172 created
>>
>> oscar.
>>
>>
>> Johan Compagner wrote:
>> >
>> > please make a jira issue for this. Then i will look at it asap.
>> >
>> > johan
>> >
>> >
>> > On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>> >>
>> >>
>> >> I'm reAttaching a component doing something like:
>> >>
>> >> MyFooPanel p1 = new MyFooPanel(this, "panel";);
>> >> MyBarPanel p2 = new MyBarPanel(this, "panel");
>> >> p1.reAttach();
>> >>
>> >> When I try to restore to the initial page version I found that the
>> >> component
>> >> with id "panel" is not a children component of the page.
>> >>
>> >> I have investigated it and I think it is because when the component is
>> >> reAttached the order in which the changes are added to the ChangesList
>> >> is:
>> >> - Add p2.
>> >> - Remove p1.
>> >>
>> >> When the initial version is restored the undo functionality is done in
>> >> reverse mode like,
>> >> - Add p1.
>> >> - Remove p2.
>> >>
>> >> The problem is p1 and p2 have the same id, so when p2 is removed what
>> is
>> >> removing is p1 that has just added.
>> >>
>> >> Oscar.
>> >> --
>> >> View this message in context:
>> >>
>> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7892497
>> >> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>> >>
>> >>
>> >
>> >
>>
>> --
>> View this message in context:
>> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7895031
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>>
> 
> 

-- 
View this message in context: http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7909819
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: Component reAttach and versioning

Posted by Johan Compagner <jc...@gmail.com>.
thx i am looking at it now.

How it is get undone is the right way.
You do exactly the reverse way of how the undo able actions are added.
So that is fine.
Then i can fix it i guess in MarkupContainer.add(Component) by calling
addedComponent a bit later
(after the if(replaces != null))

but i think the real problem is in this code:

private final int children_indexOf(Component<?> child)
    {
        if (children instanceof Component)
        {
            if (((Component)children).getId().equals(child.getId()))
            {
                return 0;
            }
        }
        else
        {
            if (children != null)
            {
                final Component[] components = (Component[])children;
                for (int i = 0; i < components.length; i++)
                {
                    if (components[i].getId().equals(child.getId()))
                    {
                        return i;
                    }
                }
            }
        }
        return -1;
    }

especially the 2 ifs:

if (components[i].getId().equals(child.getId()))

That is not right in the first place. If you have this:

MyFooPanel p1 = new MyFooPanel(this, "panel";);
MyBarPanel p2 = new MyBarPanel(this, "panel");


then p2 is the one that is in the parent. (p1 is in standby...)

if you do then this:

this.remove(p1)

Then what now happens is that p2 is removed!
Thats wrong in my eyes. That shouldn't happen you ask for p1 to get
removed..
you don't ask for remove("panel");

So i will fix that that indexOf(component) will not look at the id but just
at the identity of the component.
Do anybody see a problem with this?

should i also fix the order of which these 2 methods are called:

addedComponent(child);
removedComponent(replaced);

i thnk it is more logical to do it the other way:

removedComponent(replaced);
addedComponent(child);

because you can't add one if you don't remove one first...
Also here i don't think this will cause a problem.

johan



On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>
>
> Jira issue WICKET-172 created
>
> oscar.
>
>
> Johan Compagner wrote:
> >
> > please make a jira issue for this. Then i will look at it asap.
> >
> > johan
> >
> >
> > On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
> >>
> >>
> >> I'm reAttaching a component doing something like:
> >>
> >> MyFooPanel p1 = new MyFooPanel(this, "panel";);
> >> MyBarPanel p2 = new MyBarPanel(this, "panel");
> >> p1.reAttach();
> >>
> >> When I try to restore to the initial page version I found that the
> >> component
> >> with id "panel" is not a children component of the page.
> >>
> >> I have investigated it and I think it is because when the component is
> >> reAttached the order in which the changes are added to the ChangesList
> >> is:
> >> - Add p2.
> >> - Remove p1.
> >>
> >> When the initial version is restored the undo functionality is done in
> >> reverse mode like,
> >> - Add p1.
> >> - Remove p2.
> >>
> >> The problem is p1 and p2 have the same id, so when p2 is removed what
> is
> >> removing is p1 that has just added.
> >>
> >> Oscar.
> >> --
> >> View this message in context:
> >>
> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7892497
> >> Sent from the Wicket - Dev mailing list archive at Nabble.com.
> >>
> >>
> >
> >
>
> --
> View this message in context:
> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7895031
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: Component reAttach and versioning

Posted by Oscar Bueno <os...@isencia.com>.
Jira issue WICKET-172 created

oscar.


Johan Compagner wrote:
> 
> please make a jira issue for this. Then i will look at it asap.
> 
> johan
> 
> 
> On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>>
>>
>> I'm reAttaching a component doing something like:
>>
>> MyFooPanel p1 = new MyFooPanel(this, "panel";);
>> MyBarPanel p2 = new MyBarPanel(this, "panel");
>> p1.reAttach();
>>
>> When I try to restore to the initial page version I found that the
>> component
>> with id "panel" is not a children component of the page.
>>
>> I have investigated it and I think it is because when the component is
>> reAttached the order in which the changes are added to the ChangesList
>> is:
>> - Add p2.
>> - Remove p1.
>>
>> When the initial version is restored the undo functionality is done in
>> reverse mode like,
>> - Add p1.
>> - Remove p2.
>>
>> The problem is p1 and p2 have the same id, so when p2 is removed what is
>> removing is p1 that has just added.
>>
>> Oscar.
>> --
>> View this message in context:
>> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7892497
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>>
> 
> 

-- 
View this message in context: http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7895031
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: Component reAttach and versioning

Posted by Johan Compagner <jc...@gmail.com>.
please make a jira issue for this. Then i will look at it asap.

johan


On 12/15/06, Oscar Bueno <os...@isencia.com> wrote:
>
>
> I'm reAttaching a component doing something like:
>
> MyFooPanel p1 = new MyFooPanel(this, "panel";);
> MyBarPanel p2 = new MyBarPanel(this, "panel");
> p1.reAttach();
>
> When I try to restore to the initial page version I found that the
> component
> with id "panel" is not a children component of the page.
>
> I have investigated it and I think it is because when the component is
> reAttached the order in which the changes are added to the ChangesList is:
> - Add p2.
> - Remove p1.
>
> When the initial version is restored the undo functionality is done in
> reverse mode like,
> - Add p1.
> - Remove p2.
>
> The problem is p1 and p2 have the same id, so when p2 is removed what is
> removing is p1 that has just added.
>
> Oscar.
> --
> View this message in context:
> http://www.nabble.com/Component-reAttach-and-versioning-tf2827369.html#a7892497
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>