[google-web-toolkit] [EMAIL PROTECTED] commented on revision r4248.
Details are at
http://code.google.com/p/google-web-toolkit/source/detail?r=4248
Score: Negative
General Comment:
I like the display improvements, but the tests need work. See below.
Line-by-line comments:
File:
/branches/1_6_datepicker/reference/code-museum/src/com/google/gwt/museum/client/defaultmuseum/VisualsForDatePicker.java
(r4248)
===============================================================================
Line 62: event.getHighlighted().setYear(1);
-------------------------------------------------------------------------------
If this does fail, it will be subtle and confusing, and you could even
imagine it not showing up at all right away. How about:
Date d = event.getHighlighted();
d.setYear(1);
if (d.equals event.getHighlighted()) {
Window.alert("oh shit!");
}
Line 63: picker.getHighlightedDate().setYear(1);
-------------------------------------------------------------------------------
This check belongs in DateBoxTest, not here:
Date d = picker.getHighlightedDate();
d.setYear(1);
assertFalse("picker highlighted date should not change",
d.equals(picker.getHighlightedDate()));
Respond to these comments at
http://code.google.com/p/google-web-toolkit/source/detail?r=4248
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
