What are your thoughts on this getting into 2.5 vs. 2.5.1? I'd like to get it into 2.5, but I'm a bit worried that we may not come to consensus in time..
http://gwt-code-reviews.appspot.com/1619803/diff/7001/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/7001/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode108 user/src/com/google/gwt/user/client/ui/ValueListBox.java:108: * include {@code null} in {@code newValues}. I don't think it's clear that either Renderers or KeyProviders must support null values. If this is the case, let's update the doc in both of those interfaces. http://gwt-code-reviews.appspot.com/1619803/diff/7001/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode127 user/src/com/google/gwt/user/client/ui/ValueListBox.java:127: setValue(null); On 2011/12/21 14:10:55, stephenh wrote:
> I still believe we shouldn't call setValue here
Oh, right, agreed, sorry, I forgot about this. I'll change it.
Looks like this hasn't been fixed as yet? If we're introducing an uninitialized state, I'd agree that it should not be set to initialized until there's a call to setValue by the user (not by the implementation code). Also, i'd feel more comfortable if we used a special marker object (EMPTY_OBJ = new Object()) instead of null; that way, we'd be less likely to run into NPEs. http://gwt-code-reviews.appspot.com/1619803/diff/7001/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode138 user/src/com/google/gwt/user/client/ui/ValueListBox.java:138: * is not already there. Clarify that this applies for the null value as well. http://gwt-code-reviews.appspot.com/1619803/diff/7001/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode146 user/src/com/google/gwt/user/client/ui/ValueListBox.java:146: public void setValue(T value, boolean fireEvents) { I know we can't really change it now, but we should have made it an error if setValue was called before setAcceptableValues was called - i.e. this method should have thrown an exception. It would have been much clearer to force the user to call setAcceptableValues and update the list, as opposed to the auto-update behavior that we have here.. http://gwt-code-reviews.appspot.com/1619803/diff/7001/user/src/com/google/gwt/user/client/ui/ValueListBox.java#newcode147 user/src/com/google/gwt/user/client/ui/ValueListBox.java:147: hasBeenSet = true; I wonder if we now need a clear() method - there's really no way to get the ListBox back to it's initial state after setValue has been called.. http://gwt-code-reviews.appspot.com/1619803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
