Re: Re: [PEPr] Proposal finished HTML::HTML_QuickForm_advmultiselect

Justin Patrin wrote:

1) please don't use @ (@$param2[$key]) Better would be to do this at the top: if (!is_array($param2)) { $param2 = array(); } Then use isset($param2[$key]) 2) Why are you setting the params in the parent::load call? Shouldn't that just be parent::load($options, $param1, $param2, $param3, $param4); damn copy/paste :-[ thx!

3) IMHO this function is really not needed. I agree with the previous posters about this. If you want to include attributes for your options, write a simple loop yourself and add options each with attributes. Putting your attributes in a big array and sending it in is bound to have some kind of logic to put them in that array. Replace that loop with a loop around addOption and you're none the worse for wear. Well it seems you are not alone to think that this function is not needed.

+1 to remove

I guarantee that this is going to confuse users rather than empower them. +1 to use old implementation then

To reiterate, I like both solutions but I think they should be seperate solutions. You should be using one or the other, not both at once. Both are useful in different situations. If i used this implementation , it was to solve a problem that is not really a problem.

If you have no javascript browser compliant or just disabled, then you should not use the advmultiselect widget ! THANKS to all your feedback Justin ! i appreciated a lot :o) Laurent