ok we can change that.
A bit strange naming we have then
is null Not valid means that there can be a null in the options.
So null is a valid choice because it is a choice in the options
But you have to configure it if it wasn't a valid choice...

johan


On 10/10/06, Matej Knopp <[EMAIL PROTECTED]> wrote:

Okay, let me give you an example.

Say you have choices Boolean[] { null, True, False }

You make a RadioChoice with those options and null set as not valid
(because it's already in choices).

You set model object to null, so you want the null radio to be
preselected.

Now look at RadioChoice#onComponentTagBody, line 431

final String selected = getValue();

getValue() calls getModelValue(), which in AbstractSingleSelectChoice
looks like this:
final T object = getModelObject();
if (object != null)
{
        int index = getChoices().indexOf(object);
        return getChoiceRenderer().getIdValue(object, index);
}
return NO_SELECTION_VALUE;

So for null model object the string selected returns -1.

Later, in RadioChoice#onComponentTagBody, isSelected is called to find
out, whether the currently processed item is selected (line 460).

(isSelected(choice, index, selected)

isSelected compares string selected to
getChoiceRenderer().getIdValue(object, index).

selected contains -1 (returned by getModelObject()).
getChoiceRenderer().getIdValue(object, index) returns 0, because null is
the first object in selection.

So the result is that choice with value null is not preselected.

Does this seem ok? To me it seems more like a bug.

Again, shouldn't the AbstractSingleSelectChoice.getModelObject() look
like this?

final T object = getModelObject();
if (object != null || !isNullValid())
{
        int index = getChoices().indexOf(object);
        return getChoiceRenderer().getIdValue(object, index);
}
return NO_SELECTION_VALUE;


-Matej

Johan Compagner wrote:
> what does it matter what id the choice generates for the html option?
> It shouldn't matter at all. In the model you will be getting an null.
>
> It is just an id between html and the component
> Not the component and the model (where your code resides)
>
> johan
>
>
> On 10/9/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
>>
>> Well, yes, i agree. The problem is that wicket won't allow me to
>> generate id for my null value. Look at
>>
>> look at AbstractSingleSelectChoice#getModelValue:
>>
>> public final String getModelValue()
>>         {
>>                 final T object = getModelObject();
>>                 if (object != null)
>>                 {
>>                         int index = getChoices().indexOf(object);
>>                         return getChoiceRenderer().getIdValue(object,
>> index);
>>                 }
>>                 return NO_SELECTION_VALUE;
>>         }
>>
>> So it always returns -1 for null, even if null is not allowed, meaning
>> that I want my choice renderer to render id for null.
>>
>> Shoudln't it be more like
>>
>> public final String getModelValue()
>>         {
>>                 final T object = getModelObject();
>>                 if (!isNullValid() || object != null)
>>                 {
>>                         int index = getChoices().indexOf(object);
>>                         return getChoiceRenderer().getIdValue(object,
>> index);
>>                 }
>>                 return NO_SELECTION_VALUE;
>>         }
>>
>> ?
>>
>> -Matej
>>
>>
>> Johan Compagner wrote:
>> > this: new Boolean[] { null, True, False }.
>> >
>> > is not the same as null is a valid value!
>> > Because you really specify the null as a thing you can choose.
>> >
>> > isNullValid is more: new Boolean[] { True, False }.
>> > and then if isNullValid() == true then we (the framework) generate
the
>> -1
>> > value..
>> > So if you just specify it your self in the choices then you
completely
>> > do it
>> > yourself.
>> >
>> > johan
>> >
>> >
>> > On 10/7/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
>> >>
>> >> Say you have list of options - new Boolean[] { null, True, False }.
>> >> so Null is a valid value. Why don't we allow choice renderer to
assign
>> >> id value for null in this case?
>> >>
>> >> I think when isNullValid() is true, we should allow choice renderer
to
>> >> generate id even for null value (because it's valid). If null is
_not_
>> >> valid, then we should either return "" or -1.
>> >>
>> >> Does this sound sane?
>> >>
>> >> -Matej
>> >>
>> >> Johan Compagner wrote:
>> >> > i think we can drop that NO_SELECTION_VALUE
>> >> > and just return "" for it in AbstractSingleSelectChoice
>> >> > so that it is consistent with what ChoiceRenderer does
>> >> > Calling choicerender directly doesn't seem to be a good choice
>> >> > because that is just a default implementation..
>> >> >
>> >> > johan
>> >> >
>> >> > On 10/6/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> There is a inconsistency of how null is handled in
>> >> >> ChoiceRenderer#getIdValue and
>> >> AbstractSingleSelectChoice#getModelValue.
>> >> >>
>> >> >> ChoiceRenderer renders either "" as id, or index under which the
>> null
>> >> >> value is in choices list.
>> >> >>
>> >> >> AbstractSingleSelectChoice#getModelValue returns always -1 for
>> null.
>> >> >>
>> >> >> The outcome is that if RadioChoice has item with null value and
the
>> >> >> model object is also null, the item is not pre-selected.
>> >> >>
>> >> >> (because getValue() is compared to
>> >> >> getChoiceRenderer().getIdValue(choice, 0))
>> >> >>
>> >> >> It is a bug, but I'm not sure what would be the best way to fix
it.
>> >> >>
>> >> >> Perhaps when AbstractSingleSelectChoice#isNullValid() is true the
>> >> >> getModelValue should call choiceRenderer.getIdValue(object,
index)
>> >> even
>> >> >> for null objects?
>> >> >>
>> >> >> WDYT?
>> >> >>
>> >> >> -Matej
>> >> >>
>> >> >
>> >>
>> >>
>> >
>>
>>
>


Reply via email to