http://gwt-code-reviews.appspot.com/1099801/diff/26001/27003 File user/src/com/google/gwt/text/client/NumberFormatRenderer.java (right):
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27003#newcode31 user/src/com/google/gwt/text/client/NumberFormatRenderer.java:31: this(NumberFormat.getDecimalFormat()); The javadoc on this says it uses the format for the "default locale." That's no good, right? The default case should use the user's locale. That's why DateTimeFormatRenderer defaults to DateTimeFormat.getFormat(PredefinedFormat.DATE_SHORT) http://gwt-code-reviews.appspot.com/1099801/diff/26001/27004 File user/src/com/google/gwt/uibinder/elementparsers/DateLabelParser.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27004#newcode36 user/src/com/google/gwt/uibinder/elementparsers/DateLabelParser.java:36: static final String NO_TIMEZONE_WITHOUT_SPECIFIED_FORMAT = "May not specify a time zone if no format is given."; Why are these not private? http://gwt-code-reviews.appspot.com/1099801/diff/26001/27009 File user/src/com/google/gwt/user/client/ui/DateLabel.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27009#newcode25 user/src/com/google/gwt/user/client/ui/DateLabel.java:25: * A ValueLabel that uses {...@link DateTimeFormatRenderer}. Should add javadoc for the predefined format stuff, whatever non-obvious things the custom parser does. http://gwt-code-reviews.appspot.com/1099801/diff/26001/27009#newcode33 user/src/com/google/gwt/user/client/ui/DateLabel.java:33: public DateLabel(DateTimeFormat format) { This is bad, how can I give this thing a renderer? Also, tying to a specific implementation of renderer rather than to the parameterized interface (in this case, Renderer<Date>) is odd. If you're trying to make life easier in ui.xml files, just make the custom parser emit new DateLabel(new DateTimeFormatRenderer(whateverFormat)) http://gwt-code-reviews.appspot.com/1099801/diff/26001/27012 File user/src/com/google/gwt/user/client/ui/LabelBase.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27012#newcode83 user/src/com/google/gwt/user/client/ui/LabelBase.java:83: * {...@inheritdoc} We don't seem to do the inheritdoc thing http://gwt-code-reviews.appspot.com/1099801/diff/26001/27013 File user/src/com/google/gwt/user/client/ui/NumberLabel.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27013#newcode32 user/src/com/google/gwt/user/client/ui/NumberLabel.java:32: public NumberLabel(NumberFormat format) { Again, should be Renderer<Number> http://gwt-code-reviews.appspot.com/1099801/diff/26001/27014 File user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java (right): http://gwt-code-reviews.appspot.com/1099801/diff/26001/27014#newcode39 user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java:39: public class SimpleCheckBox extends FocusWidget implements HasName, HasValue<Boolean>, IsEditor<LeafValueEditor<Boolean>> { All this copy paste kills me, but IIRC making CheckBox use this class is impractical. http://gwt-code-reviews.appspot.com/1099801/diff/26001/27014#newcode235 user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java:235: // overrides in RadioButton RadioButton doesn't override this, copy paste error. I don't see SimpleRadioButton in this patch, shouldn't I? http://gwt-code-reviews.appspot.com/1099801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
