Just though about one alternative design: how about special-casing
'null', but only for *not* adding a value to the ListBox if the value is
not found in the acceptable values, and then set the selectedIndex to -1
(which browsers interpret as selecting no value // would have to
double-check on IE6 though; seems to work OK in IE8 and IE10, and
according to HTML5, so it's a "future proof" solution).
I.e. in updateListBox:
Object key = keyProvider.getKey(value);
if (index == null) {
if (value != null) {
addValue(value);
index = valueKeyToIndex.get(key);
assert index != null;
} else {
index = -1;
}
}
getListBox().setSelectedIndex(index);
(note, the above code also removes an unneeded second call to
valueKeyToIndex when the first one already returned a non-null value)
Otherwise, see my comments below.
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java
File user/src/com/google/gwt/user/client/ui/ValueListBox.java (right):
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode54
user/src/com/google/gwt/user/client/ui/ValueListBox.java:54: private
boolean hasBeenSet;
Maybe 'valueInitialized' (or possibly 'valueExplicitlySet' or
'explicitValue') would be clearer; what we want here is to differentiate
between 'null' as the default value for an object reference field, and
'null' as an explicitly set value.
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode101
user/src/com/google/gwt/user/client/ui/ValueListBox.java:101: *
Javadoc is HTML, these blank lines should be <p>s instead.
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode102
user/src/com/google/gwt/user/client/ui/ValueListBox.java:102: * If the
current value has been explicitly set (by a call to setValue), and
{@link #setValue}
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode106
user/src/com/google/gwt/user/client/ui/ValueListBox.java:106: * If the
current value has not been set (no call to setValue has been made),
Replace "If the current value has not been set" with "Otherwise" to make
the sentence simpler (and a bit easier to read).
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode107
user/src/com/google/gwt/user/client/ui/ValueListBox.java:107: * then the
current value will become the first entry of {@code newValues}.
How about: "Otherwise (no call to {@link #setValue} has been made yet),
the first entry of {@code newValues} will be used as the ValueListBox's
value."
(and remove the "This will prevent an unnecessary ..." sentence: it's
only understandable by people having issues with the *current*
implementation; if you want to hint at the change, then say instead
something along the lines of "In a previous version, this would have
inserted a {@code null} value at the end of the acceptable values, as if
you had called {@code setValue(null)} prior to {@code
setAcceptableValues}").
http://gwt-code-reviews.appspot.com/1619803/diff/1009/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode130
user/src/com/google/gwt/user/client/ui/ValueListBox.java:130:
setValue(null);
Directly assign this.value to avoid the updateListBox() call from
setValue().
http://gwt-code-reviews.appspot.com/1619803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
