Comment on DateBox:
Would be cool if there would be a way to get the value of the DateBox.
Right now I struggle to find a way to listen to value changes and get
the value afterwards.
I tried to listen to DatePicker and TextBox changes, but it's very
complicated to find the proper value there.
Proposal: Add ChangeListener support and add a "Date getValue()"
method.
On 20 Nov., 04:26, Ray Ryan <[EMAIL PROTECTED]> wrote:
> General comments
>
> It seems like there is a lot of overlap between DatePicker and CalendarView.
> Should the methods in DatePicker that are redundant with CalendarView
> methods be stricken, given DatePicker#getCalendarView? Or, should
> getCalendarView perhaps go away, or become protected or package--is it
> really useful/safe for clients to talk to it directly?
>
> ElementMapper.java
>
> We need ElementMapperTest
>
> The param type MappedType should be a single letter, perhaps U
>
> What's going on Iterator#remove? This class claims to work on UIObjects, but
> here you are casting to Widget and asserting that parent is an HTMLTable.
>
> CalendarModel.java
>
> We need CalendarModelTest
>
> So this class has the notion of the current date shared by the various
> moving parts, and it does formatting, yes? Could punch up its javadoc to
> that effect.
>
> What is that no-op refresh() method there for?
>
> DatePicker.java
>
> 1. It's confusing that there is a CalendarView named calendar, and a
> CalendarModel named model. I'd expect the view to be named view, and the
> model to be named calendar.
>
> 2. It seems like many fields here could be final.
>
> 3. The Css interface is protected, but I though we were going to make it
> private, or at least package protected. It is also featured in a ton of
> public methods and constructors. That's no good.
>
> If it does wind up staying visible, having a public static setDefaultCss
> method is a bad idea. Allowing it to be overridden per instance is plenty.
>
> 4. getDateShown() is implemented as getModel().getCurrentMonth(). Isn't that
> a bug? If not, shouldn't the method be called getMonthShown()?
>
> 5. It seems like these methods:
>
> public final void addStyleToVisibleDate(Date visibleDate, String
> styleName) {
> calendar.addStyleToDate(visibleDate, styleName);
> }
>
> public final void addStyleToVisibleDates(Iterable<Date> visibleDates,
> String styleName) {
> getCalendarView().addStyleToDates(visibleDates, styleName);
> }
>
> could instead be
>
> public final void addStyleToVisibleDates(String styleName, Date...
> visibleDates) {
> addStyleToVisibleDates(styleName, Arrays.asList(visibleDates));
> }
>
> public final void addStyleToVisibleDates(String styleName, Iterable<Date>
> visibleDates) {
> getCalendarView().addStyleToDates(visibleDates, styleName);
> }
>
> Even if not, why is one using calendar and the other using
> getCalendarView()?
>
> And note that the remove method is not parallel to these--there is only one.
>
> 6. getHighlightedDate() javadoc should explain diff between selected and
> highlighted. class doc should probably do the same.
>
> 7. getGlobalStyleOfDate() What is a global style? Is this a GWT wide
> concept? If not, needs explicating.
>
> 8. showDate() Need to explain how this differs from setValue(), and link
> between the two methods.
>
> 9. What is the package private setModel(CalendarModel) needed for?
>
> 10. Some methods are final, and some aren't. It seems very arbitrary. And we
> should be documenting final methods to explain why.
>
> 11. This seems harsh (the assert, I mean):
>
> public final void setEnabledOnVisibleDate(Date date, boolean enabled) {
>
> assert isDateVisible(date) : date
>
> + " cannot be enabled or disabled as it is not visible";
>
> getCalendarView().setDateEnabled(date, enabled);
>
> }
>
> What happens if they disable a date, the user scrolls it away, and then
> scrolls it back? Do they have to listen for these scroll events and then
> reapply the disables? Is this a method people actually asked for, can we get
> rid of it?
>
> 12. setAllowableDates says it is "not yet implemented for the default case".
> Why is it here, then?
>
> DateBox.java
>
> 1. Constitutent fields (box, picker, popup) should be final
>
> DefaultMonthSelector.java
>
> JavaDoc says that it is "not part of the public API". If it's a public class
> and can be used as an arg to a protected constructor, it's public, let's be
> honest here. Perhaps you meant "not extensible"? As for "please feel to copy
> it...", that's a pretty severe mixed message.
>
> I don't really see what we're gaining by encouraging people to copy it and
> then claiming that it's not public and subject to change. If they copy it,
> we will break them. We're providing a protected constructor that accepts
> this. That's public api. We need to make up our minds--providing your own
> CalendarView and MonthSelector is supported, or it isn't.
>
> MonthSelector.java
>
> More of the same. This class contributes basically nothing. We're trying to
> allow custom MonthSelector and at the same time trying to avoid it. We need
> to make up our minds.
>
> CalendarUtil.java
>
> Some of the methods in this public class are not public, seemingly
> arbitrarily. Is there a real reason we don't want people to call isWeekend
> or hasTime? Why is resetTime private?
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
