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.";
On 2010/11/18 05:46:19, rjrjr wrote:
Why are these not private?
Testing.
(I believe I copied this from another class but can find it back; but
clearly other classes could benefit from it: LayoutPanelParserTest for
instance could use the constants from LayoutPanelParser instead of
duplicating the string values)
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27005
File
user/src/com/google/gwt/uibinder/elementparsers/NumberLabelParser.java
(right):
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27005#newcode70
user/src/com/google/gwt/uibinder/elementparsers/NumberLabelParser.java:70:
writer.getOracle().findType(CurrencyData.class.getCanonicalName()));
On 2010/11/15 22:14:37, jat wrote:
How do you get a CurrencyData instance from the ui.xml file?
using a field reference:
<ui:with field='foo' type='my.CurrencyDataProvider' />
<g:NumberLabel currencyData='{foo.myCurrency}' />
This was only added for completeness.
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#newcode33
user/src/com/google/gwt/user/client/ui/DateLabel.java:33: public
DateLabel(DateTimeFormat format) {
On 2010/11/18 05:46:19, rjrjr wrote:
This is bad, how can I give this thing a renderer?
My reasoning was that if you do have a renderer, you don't need a
DateLabel, just use a ValueLabel<Date> instead. Maybe it should just be
documented?
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}
On 2010/11/18 05:46:19, rjrjr wrote:
We don't seem to do the inheritdoc thing
It was in Label.
http://code.google.com/p/google-web-toolkit/source/browse/trunk/user/src/com/google/gwt/user/client/ui/Label.java?r=9187#319
I just used Eclipse's "extract superclass" tool.
I've removed it.
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) {
On 2010/11/18 05:46:19, rjrjr wrote:
Again, should be Renderer<Number>
Same rationale as for DateLabel: if you have a Renderer<Number> at hand,
just use a ValueLabel<Number>, the whole point of DateLabel and
NumberLabel is to tie them to specific renderers.
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#newcode235
user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java:235: //
overrides in RadioButton
On 2010/11/18 05:46:19, rjrjr wrote:
RadioButton doesn't override this, copy paste error.
I don't see SimpleRadioButton in this patch, shouldn't I?
Ah oops! Maybe we should just stick with TakesValue instead of HasValue
then, it would make things much simpler! ;-)
(I also note that SimpleCheckBox/SimpleRadioButton don't have a setName
like CheckBox/RadioButton)
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27025
File
user/test/com/google/gwt/uibinder/elementparsers/DateLabelParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27025#newcode230
user/test/com/google/gwt/uibinder/elementparsers/DateLabelParserTest.java:230:
public void testChokeOnUnknownPredefinedFormat() throws SAXException {
On 2010/11/15 22:14:37, jat wrote:
Should there be a test that shows predefinedFormat working and having
the right
output?
Added a testHappyWithPredefinedFormat and testHappyWithTimeZoneOffset.
(I wonder whether there should be explicit tests for other things, given
that they're implicitly tested in other tests; for instance, timezone is
tested in testHappyWithSubclassWithDateTimeFormatAndTimeZoneConstructor,
and format is tested in all testHappy* tests except
testHappyWithNoFormat and testHappyWithPredefinedFormat)
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27026
File
user/test/com/google/gwt/uibinder/elementparsers/NumberLabelParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1099801/diff/26001/27026#newcode145
user/test/com/google/gwt/uibinder/elementparsers/NumberLabelParserTest.java:145:
b.append("<g:NumberLabel predefinedFormat='CURRENCY'>");
On 2010/11/15 22:14:37, jat wrote:
I like the parallelism with DateTimeFormat, and we probably should
make a
similar change to it that includes a PredefinedFormat enum.
I agree. Do you want me to create another review with this change? (and
update this one rely on it; or rather include the NumberFormat change
right into this review?)
There's a difference with DateLabelParser though, in that
PredefinedFormat.CURRENCY would still have to be special-cased in the
NumberLabelParser, at least when used with a currency code.
http://gwt-code-reviews.appspot.com/1099801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
