[Yanel-dev] Re: [Yanel-commits] rev 45177 - public/yanel/contributions/resources/creatable-modifiable-deletable-v3/src/java/org/wyona/yanel/impl/jelly/tags

[Michael Wechner]

Guillaume Déflache wrote:
> Michael Wechner schrieb:
>> Guillaume Déflache wrote:
> [...]
>>>> public/yanel/contributions/resources/creatable-modifiable-deletable-v3/src/java/org/wyona/yanel/impl/jelly/tags/CheckboxTag.java    
>>>> 2009-10-24 22:33:38 UTC (rev 45176)
>>>> +++ 
>>>> public/yanel/contributions/resources/creatable-modifiable-deletable-v3/src/java/org/wyona/yanel/impl/jelly/tags/CheckboxTag.java    
>>>> 2009-10-24 22:34:45 UTC (rev 45177)
>>>> @@ -31,6 +31,12 @@
>>>>                  attributes.addAttribute(XHTML_NAMESPACE, "", 
>>>> "type", "CDATA", "checkbox");
>>>>                  attributes.addAttribute(XHTML_NAMESPACE, "", 
>>>> "name", "CDATA", item.getName());
>>>>                  attributes.addAttribute(XHTML_NAMESPACE, "", 
>>>> "value", "CDATA", selection[i].getValue());
>>>> +                if (selection[i].getValue().equals("true")) {
>>>> +                    log.debug("Checked!");
>>>> +                    attributes.addAttribute(XHTML_NAMESPACE, "", 
>>>> "checked", "CDATA", "true");
>>>> +                } else {
>>>> +                    log.debug("NOT Checked!");
>>>> +                }
>>>>                                   if (isSelected(selection[i])) {
>>>>                      attributes.addAttribute(XHTML_NAMESPACE, "", 
>>>> "checked", "CDATA", "checked");
>>>>
>>>
>>> This does not seem to work because in some cases the "checked" 
>>> attribute is added two times (see exception at the bottom of the 
>>> message).
>> in which cases? And how can I reproduce your exception?
>
> Well, from the code above when selection[i].getValue().equals("true") 
> && isSelected(selection[i]) obviously, although I am not sure what 
> that means in practice...

that's the problem. Me neither. I think it's quite confusing that a 
checkbox needs to be a multiple item in the first place anyway.

I think we should introduce an individual checkbox item at some later stage
>
> So I guess we should use the following instead:
>
>                 if (isSelected(selection[i]) // checked upon creation
>                 || selection[i].getValue().equals("true") // checked 
> during filling out the form
>                 ) {
>                     attributes.addAttribute(XHTML_NAMESPACE, "", 
> "checked", "CDATA", "checked");
>                 }
>
>
> WDYT?

sounds good to me. Maybe you can give it a try within your setup and see 
if this resolves the issues which you seem to have.

Thanks

Michael
>
>>>
>>> What was the purpose of this change? Did it fix some bug?
>>
>> yes, one couldn't set a checkbox to checked from the very beginning 
>> (checkboxes are not checked by default and couldn't be set to checked 
>> upon creation
>> or rather if set to true upon creation, the checked attribute was not 
>> set).
>>>
>>> Also using "true" as a value for the "checked" attribute seems wrong 
>>> (this is not valid XHTML), why wasn't "checked" used?
>>
>> my bad, I have changed it now.
>
> OK, thanks!