You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Jean-Baptiste Quenot <jb...@apache.org> on 2006/11/16 16:09:41 UTC

Removing a row from ListView in a form

Hi,

I'm having  a very  annoying issue  with ListView  when used  in a
form,  with  reuseItems set.   Apply  the  attached patch  to  the
wicket-examples: it adds  a link to remove an item  in the list of
lines in  the forminput example.   Now start the  patched examples
and go to /wicket-examples/forminput.

Click to  remove the first line:  you will notice that  the second
line is removed!   That is the first strange  thing.  Same problem
when removing the second item.

Now another test: replace the  first line with value « Hello » and
click to remove the third line.  You will notice that the text you
have entered is reset.

Basically  I  have  the  feeling  that  ListView  is  broken  when
reuseItems is set and rows are deleted.

How can we fix that?

We need  to allow to use  SubmitLink instead of Link  to make sure
form values are sent, by  creating a removeItem() method extracted
from removeLink().

But this won't be  sufficient: ListView currently removes rows and
re-populates them from  the model.  This clearly  means user input
is  lost.   Parts of  ListView  must  be rewritten,  most  notably
internalOnAttach().  Also, the rendering  of the ListView is based
on the fact  that ListItems have an id that  reflects the index in
the list.   But because the id  of a component cannot  be changed,
that clearly shows that all components must be created again.  The
render()  method has  to be  reworked,  so that  ListItem ids  are
picked from a sequence to have unique values.

Because of  the necessary  incompatible changes, I  think ListView
should be  forked, unless there  is another Wicket  component that
fits the job?

What do you think?
-- 
     Jean-Baptiste Quenot
aka  John Banana Qwerty
http://caraldi.com/jbq/

Re: Removing a row from ListView in a form

Posted by Igor Vaynberg <ig...@gmail.com>.
On 11/19/06, Jean-Baptiste Quenot <jb...@apache.org> wrote:
>
> * Igor Vaynberg:
>
> >
> > CheckGroup.convert(); CheckGroup.getConvertedValue();
>
> getConvertedInput() you mean?


yes


[...]
> Providing  access to  the  checked checkboxes  directly allows  to
> do  checkbox.getParent() to  get  the ListItem.   And getting  the
> ListItems  allows  for example  to  modify  the structure  of  the
> ListView for  adding, removing and  moving rows.  Why would  it be
> mandatory to  use a  list of  business models,  when one  wants to
> access a list of Wicket Components?


its called getModelObject() not getBusinessModelObject(), feel free to use
listitem instance as the model object.

What I'm trying to do currently with Wicket is to implement a very
> common  type of  widget  that  we are  used  to  with Cocoon:  the
> repeater,  which comes  with  built-in facilities  for removing  a
> selection of  rows, adding  rows, and moving  them.  Surprisingly,
> Wicket is not good at that.
>
> See for yourself the repeater widget on the Cocoon demo site:
>
> http://cocoon.zones.apache.org/demos/21branch/samples/blocks/forms/do-dynaRepeater.flow
>
> Do you have such an example in Wicket?



wicket works differently, you work with backing models not with components.
so instead of removing a row from the listview manually you would remove the
object from the list that backs the listview. listview is fragile, i have
said it time and time again, because it uses the index of an object in a
list as the primary key to identify the object which hardly ever works for
editing.

-igor


--
>      Jean-Baptiste Quenot
> aka  John Banana Qwerty
> http://caraldi.com/jbq/
>

Re: Removing a row from ListView in a form

Posted by Jean-Baptiste Quenot <jb...@apache.org>.
* Igor Vaynberg:

> > Also, now  if instead of removing  one row at a  time you want
> > to  remove  a selection  of  rows  using a  CheckGroup,  there
> > is  no way  to  determine  the checked  rows,  unless you  add
> > a  CheckGroup.getSelection() method  that  returns  a list  of
> > checked checkboxes, WICKET-79 provides such a feature.
>
> CheckGroup.convert(); CheckGroup.getConvertedValue();

getConvertedInput() you mean?

getConvertedInput() iterates  over all child checkboxes  (of class
Check), and it stores checkbox.getModelObject() in a List.

Basically    the   patch    in    WICKET-79    reuses   most    of
getConvertedInput()  in a  new  method  called getSelection()  but
returns a  list of  checkboxes instead of  a list  of checkboxes's
model objects, this is more generic.

Providing  access to  the  checked checkboxes  directly allows  to
do  checkbox.getParent() to  get  the ListItem.   And getting  the
ListItems  allows  for example  to  modify  the structure  of  the
ListView for  adding, removing and  moving rows.  Why would  it be
mandatory to  use a  list of  business models,  when one  wants to
access a list of Wicket Components?

What I'm trying to do currently with Wicket is to implement a very
common  type of  widget  that  we are  used  to  with Cocoon:  the
repeater,  which comes  with  built-in facilities  for removing  a
selection of  rows, adding  rows, and moving  them.  Surprisingly,
Wicket is not good at that.

See for yourself the repeater widget on the Cocoon demo site:
http://cocoon.zones.apache.org/demos/21branch/samples/blocks/forms/do-dynaRepeater.flow

Do you have such an example in Wicket?
-- 
     Jean-Baptiste Quenot
aka  John Banana Qwerty
http://caraldi.com/jbq/

Re: Removing a row from ListView in a form

Posted by Igor Vaynberg <ig...@gmail.com>.
> Also,  now  if  instead  of  removing   one  row  at  a  time  you
> want  to remove  a selection  of  rows using  a CheckGroup,  there
> is  no  way to  determine  the  checked  rows,  unless you  add  a
> CheckGroup.getSelection() method  that returns  a list  of checked
> checkboxes, WICKET-79 provides such a feature.


CheckGroup.convert(); CheckGroup.getConvertedValue();

-igor





All the best,
> --
>      Jean-Baptiste Quenot
> aka  John Banana Qwerty
> http://caraldi.com/jbq/
>

Re: Removing a row from ListView in a form

Posted by Jean-Baptiste Quenot <jb...@apache.org>.
Thanks for your answers.  However there is still some issues:

* Eelco Hillenius:

> If we use a submit link,  first the models would be updated, and
> then the link with the delete behavior would be executed.

Oh  yes, sorry  I didn't  mention: the  SubmitLink that  is likely
to  be used  for  removing  rows is  *non-validating*  ie it  sets
setDefaultProcessing()  to  false,  so  no,  the  model  is  *not*
updated.  That is the problem.   Requesting the user to have valid
values in a row to be able to delete it doesn't make sense.

Also,  now  if  instead  of  removing   one  row  at  a  time  you
want  to remove  a selection  of  rows using  a CheckGroup,  there
is  no  way to  determine  the  checked  rows,  unless you  add  a
CheckGroup.getSelection() method  that returns  a list  of checked
checkboxes, WICKET-79 provides such a feature.

All the best,
-- 
     Jean-Baptiste Quenot
aka  John Banana Qwerty
http://caraldi.com/jbq/

Re: Removing a row from ListView in a form

Posted by Jean-Baptiste Quenot <jb...@apache.org>.
* Eelco Hillenius:

> > Click  to remove  the first  line:  you will  notice that  the
> > second  line is  removed!  That  is the  first strange  thing.
> > Same problem when removing the second item.
>
> It only looks like that. The childs should have been removed. If
> you update, you'll find the changes.

Noticed you fixed it in ListView,  and updated the example as well
to include the remove link.  The result looks better.  But...

> > We need to allow to use SubmitLink instead of Link.
>
> That might work.

Please find attached a patch that replaces the Link in the example
with a SubmitLink.  Note that I setDefaultFormProcessing(false) on
purpose, and  use a  RequiredTextField, to  illustrate that  it is
allowed to remove an empty line.

Before clicking the Remove button, please change one of the values
of  the other  lines.   You will  notice that  the  right line  is
removed, even if the field is  empty, but the other changed values
are lost.
-- 
     Jean-Baptiste Quenot
aka  John Banana Qwerty
http://caraldi.com/jbq/

Re: Removing a row from ListView in a form

Posted by Igor Vaynberg <ig...@gmail.com>.
no, they both do exactly what they were designed to do. sounds like you need
to create a subclass of repeatingview for your usecase.

-igor


On 11/20/06, Jean-Baptiste Quenot <jb...@apache.org> wrote:
>
> * Igor Vaynberg:
>
> > think of RepeatingView as an unmanaged ListView. you are in full
> > control over how the children are managed.
>
> I just tried  RepeatingView, and well, the problem is  that as you
> say it is totally unmanaged, ie it does not handle a backing List.
>
> So  either we  need  to change  ListView, or  we  have to  improve
> RepeatingView to handle a backing list.
> --
>      Jean-Baptiste Quenot
> aka  John Banana Qwerty
> http://caraldi.com/jbq/
>

Re: Removing a row from ListView in a form

Posted by Jean-Baptiste Quenot <jb...@apache.org>.
* Igor Vaynberg:

> think of RepeatingView as an unmanaged ListView. you are in full
> control over how the children are managed.

I just tried  RepeatingView, and well, the problem is  that as you
say it is totally unmanaged, ie it does not handle a backing List.

So  either we  need  to change  ListView, or  we  have to  improve
RepeatingView to handle a backing list.
-- 
     Jean-Baptiste Quenot
aka  John Banana Qwerty
http://caraldi.com/jbq/

Re: Removing a row from ListView in a form

Posted by Igor Vaynberg <ig...@gmail.com>.
think of RepeatingView as an unmanaged ListView. you are in full control
over how the children are managed.

-igor


On 11/19/06, Jean-Baptiste Quenot <jb...@apache.org> wrote:
>
> * Eelco Hillenius:
>
> > > unless there is another Wicket component that fits the job?
> >
> > Wicket-exention's repeater  package was  set up  to be  a better
> > ListView.  You might try that.
>
> Mmm, that seems interesting indeed:  RepeatingView.newChildId() is
> exactly  what I  rewrote  for ListView:  generating children  with
> unique IDs.
>
> I will try to reconsider the whole issue using RepeatingView.
>
> Thanks,
> --
>      Jean-Baptiste Quenot
> aka  John Banana Qwerty
> http://caraldi.com/jbq/
>

Re: Removing a row from ListView in a form

Posted by Jean-Baptiste Quenot <jb...@apache.org>.
* Eelco Hillenius:

> > unless there is another Wicket component that fits the job?
>
> Wicket-exention's repeater  package was  set up  to be  a better
> ListView.  You might try that.

Mmm, that seems interesting indeed:  RepeatingView.newChildId() is
exactly  what I  rewrote  for ListView:  generating children  with
unique IDs.

I will try to reconsider the whole issue using RepeatingView.

Thanks,
-- 
     Jean-Baptiste Quenot
aka  John Banana Qwerty
http://caraldi.com/jbq/

Re: Removing a row from ListView in a form

Posted by Eelco Hillenius <ee...@gmail.com>.
> Click to  remove the first line:  you will notice that  the second
> line is removed!   That is the first strange  thing.  Same problem
> when removing the second item.

It only looks like that. The childs should have been removed. If you
update, you'll find the changes.

> Now another test: replace the  first line with value «Hello» and
> click to remove the third line.  You will notice that the text you
> have entered is reset.

Yep, that is true. Normal links are used, and thus any other fields
are not included in the submit.

> We need  to allow to use  SubmitLink instead of Link.

That might work.

> But this won't be  sufficient: ListView currently removes rows and
> re-populates them from  the model.  This clearly  means user input
> is  lost.

I don't think so. If we use a submit link, first the models would be
updated, and then the link with the delete behavior would be executed.

On a side note, Johan I don't remember exactly anymore why this has to
be done on object identity. This obviously doesn't work properly if
you have multiple references of the same object (or object with the
same identity) in the list. Why wouldn't just using the index work
work again?

> Because of  the necessary  incompatible changes, I  think ListView
> should be  forked

We're working on 1.3 and 2.0, for which a few breaks are permitted.

>, unless there  is another Wicket  component that
> fits the job?

Wicket-exention's repeater package was set up to be a better ListView.
You might try that.

Eelco