Wicket
  1. Wicket
  2. WICKET-466

[datetime] Make DateConverter focus on joda-time.DateTime instead of util.Date

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.3.0-beta1
    • Fix Version/s: None
    • Component/s: wicket-datetime
    • Labels:
      None

      Description

      I beleive the Wicket-datetime package was designed to be used with joda-time DateTime objects. It was also desirable to apply the "better" parsing/toString features of DateTime to util.Date objects. To that end, the attached patch improves the focus of the Converter to be DateTime oriented, while still allowing the DateTextField to hold util.Dates in it's model.

      The main idea of this patch is to make this package work with DateTime objects with very little effort on the developer's part. If they want to take advantage of the improved parsing, etc then it should take minimal effort.

      See http://www.nabble.com/-datetime--DateConverter-tf3233793.html for a brief history of this conversation.

      1. wicket-datetime.patch
        37 kB
        Patrick Angeles
      2. datetime.zip
        42 kB
        Patrick Angeles
      3. datetime_patch.txt
        27 kB
        Chuck Deal
      4. datetime_patch.txt
        24 kB
        Chuck Deal
      5. datetime_patch.txt
        24 kB
        Chuck Deal
      6. datetime_patch.txt
        20 kB
        Chuck Deal
      7. quickstart-datetimeconverter.zip
        27 kB
        Chuck Deal
      8. datetime_patch.txt
        20 kB
        Chuck Deal

        Activity

        Hide
        Martin Grigorov added a comment -

        "'scm' blame" shows that the code in this file is 1.5 years older than the last Eelco's comment.
        The used formatter is indeed Joda Time, but the input/output parameter is still j.u.Date. Maybe this is the idea - to move completely to Joda Time.

        Show
        Martin Grigorov added a comment - "'scm' blame" shows that the code in this file is 1.5 years older than the last Eelco's comment. The used formatter is indeed Joda Time, but the input/output parameter is still j.u.Date. Maybe this is the idea - to move completely to Joda Time.
        Hide
        Juergen Donnerstag added a comment -

        looking at the 1.5 code, it seems to be implemented already. At least it's using joda.

        Show
        Juergen Donnerstag added a comment - looking at the 1.5 code, it seems to be implemented already. At least it's using joda.
        Hide
        Igor Vaynberg added a comment -

        this will be done in 1.5

        Show
        Igor Vaynberg added a comment - this will be done in 1.5
        Hide
        Eelco Hillenius added a comment -

        I looks to me like this patch was in the right direction and supported working with java.util.Date (though I just did a very shallow test). I just disagreed with breaking the API without providing backwards compat and better documentation on how to upgrade. And we should probably have some tests that assure the code works for both Date and DateTime.

        If you feel like hacking around, you can definitively take look at the patch for inspiration

        Show
        Eelco Hillenius added a comment - I looks to me like this patch was in the right direction and supported working with java.util.Date (though I just did a very shallow test). I just disagreed with breaking the API without providing backwards compat and better documentation on how to upgrade. And we should probably have some tests that assure the code works for both Date and DateTime. If you feel like hacking around, you can definitively take look at the patch for inspiration
        Hide
        Ned Collyer added a comment -

        Is there much effort to get the date picker to support both Date and DateTime? (disregarding the patch, and looking at whats in the system currently).

        I'm about to hack it apart in my own source tree - but I'm not sure I know what I'm doing

        Show
        Ned Collyer added a comment - Is there much effort to get the date picker to support both Date and DateTime? (disregarding the patch, and looking at whats in the system currently). I'm about to hack it apart in my own source tree - but I'm not sure I know what I'm doing
        Hide
        Eelco Hillenius added a comment -

        Unfortunately, this patch has a few issues I disagree with.

        • It breaks the API big time, whereas I think it should have been done in such a way that it is backwards compatible.
        • It misses documentation, such as an explanation of alternatives for methods/ classes that are marked deprecated
        • I saw at least one instance of Locale.getDefault(), which should never be used with Wicket (instead in this case just use the session's locale)

        So... sorry but in the current state, I disagree with the patch.

        Show
        Eelco Hillenius added a comment - Unfortunately, this patch has a few issues I disagree with. It breaks the API big time, whereas I think it should have been done in such a way that it is backwards compatible. It misses documentation, such as an explanation of alternatives for methods/ classes that are marked deprecated I saw at least one instance of Locale.getDefault(), which should never be used with Wicket (instead in this case just use the session's locale) So... sorry but in the current state, I disagree with the patch.
        Hide
        Johan Compagner added a comment -

        eelco is a bit in hibernation...

        i wont be able to look at it the coming 6 days but i will pick up the outstanding issues after that

        Show
        Johan Compagner added a comment - eelco is a bit in hibernation... i wont be able to look at it the coming 6 days but i will pick up the outstanding issues after that
        Hide
        Ned Collyer added a comment -

        Hi, Just curious if there there any movement on this item.

        I see a patch was provided about 9 months ago - has anyone had a chance to review it?

        Show
        Ned Collyer added a comment - Hi, Just curious if there there any movement on this item. I see a patch was provided about 9 months ago - has anyone had a chance to review it?
        Hide
        Martijn Dashorst added a comment -

        Moved to next milestone release.

        Show
        Martijn Dashorst added a comment - Moved to next milestone release.
        Hide
        Patrick Angeles added a comment -

        Attaching wicket-datetime.patch as per Eelco's request...

        Not sure if I did this right so let me know if it works...

        Some notes:

        • made everything Java 1.4 friendly
        • added org.apache.wicket.calendar.form package, which carries over from wicket-calendar (this has a DateField, DatePicker, etc...) not really critical to this patch, i'm just including it for completeness
        • PatternDateConverter and StyleDateConverter are deprecated
        • Bunch of DateTextField static factory methods have been deprecated... use the constructor instead.
        Show
        Patrick Angeles added a comment - Attaching wicket-datetime.patch as per Eelco's request... Not sure if I did this right so let me know if it works... Some notes: made everything Java 1.4 friendly added org.apache.wicket.calendar.form package, which carries over from wicket-calendar (this has a DateField, DatePicker, etc...) not really critical to this patch, i'm just including it for completeness PatternDateConverter and StyleDateConverter are deprecated Bunch of DateTextField static factory methods have been deprecated... use the constructor instead.
        Hide
        Eelco Hillenius added a comment -

        Hi Patrick. Your changes sounds good, but would it be possible to make this a patch instead of a zipped project? That way it is easier for us to look at and apply the changes.

        Show
        Eelco Hillenius added a comment - Hi Patrick. Your changes sounds good, but would it be possible to make this a patch instead of a zipped project? That way it is easier for us to look at and apply the changes.
        Hide
        Patrick Angeles added a comment -

        I'm attaching an alternate implementation of DateConverter, DateLabel, DateField, etc... mostly based on Eelco's wicket-calendar code.

        Features:

        • apply timezone difference between client and server (as per original wicket-calendar code)
        • use IModels with the following underlying types: java.util.Date, org.joda.time.DateTime and org.joda.time.LocalDate.
        • No need for a separate StyleDateConverter and PatternDateConverter... can distinguish between org.joda.time.DateTImeFormat and java.text.DateFormat patterns.
        Show
        Patrick Angeles added a comment - I'm attaching an alternate implementation of DateConverter, DateLabel, DateField, etc... mostly based on Eelco's wicket-calendar code. Features: apply timezone difference between client and server (as per original wicket-calendar code) use IModels with the following underlying types: java.util.Date, org.joda.time.DateTime and org.joda.time.LocalDate. No need for a separate StyleDateConverter and PatternDateConverter... can distinguish between org.joda.time.DateTImeFormat and java.text.DateFormat patterns.
        Hide
        Gerolf Seitz added a comment -

        Dan, what version of wicket-datetime are you using? i just tried it with the latest trunk and there was no JS error here.
        the escaping of the ' doesn't work properly though, meaning that the ' are shown in the textfield even if they weren't escaped like ''.

        Show
        Gerolf Seitz added a comment - Dan, what version of wicket-datetime are you using? i just tried it with the latest trunk and there was no JS error here. the escaping of the ' doesn't work properly though, meaning that the ' are shown in the textfield even if they weren't escaped like ''.
        Hide
        Dan Syrstad added a comment -

        Joda date formats can break the generated JS because Joda allows single quotes in the string to escape text. For example, a Joda date format of "MM/dd/yyyy' 'HH:mm:ss" causes the following JS to be generated in 1.3beta3

        Wicket.DateTime.showCalendar(YAHOO.wicket.dateTextField9DpJs, YAHOO.util.Dom.get("dateTextField9").value, 'MM/dd/yyyy' 'HH:mm:ss');

        This is a JS error because there are 2 strings erroneously generated: 'MM/dd/yyyy' and 'HH:mm:ss'.

        Show
        Dan Syrstad added a comment - Joda date formats can break the generated JS because Joda allows single quotes in the string to escape text. For example, a Joda date format of "MM/dd/yyyy' 'HH:mm:ss" causes the following JS to be generated in 1.3beta3 Wicket.DateTime.showCalendar(YAHOO.wicket.dateTextField9DpJs, YAHOO.util.Dom.get("dateTextField9").value, 'MM/dd/yyyy' 'HH:mm:ss'); This is a JS error because there are 2 strings erroneously generated: 'MM/dd/yyyy' and 'HH:mm:ss'.
        Hide
        Chuck Deal added a comment -

        Just trying to keep in sync with the changes on trunk...

        Show
        Chuck Deal added a comment - Just trying to keep in sync with the changes on trunk...
        Hide
        Chuck Deal added a comment -

        Ok, I've updated to revision 533733. This is the same patch as before, but it should be much easier to apply because the name change is now included.

        Show
        Chuck Deal added a comment - Ok, I've updated to revision 533733. This is the same patch as before, but it should be much easier to apply because the name change is now included.
        Hide
        Chuck Deal added a comment -

        This patch is based upon SVN just prior to the package rename. It is the full patch (not a patch of a patch)

        Show
        Chuck Deal added a comment - This patch is based upon SVN just prior to the package rename. It is the full patch (not a patch of a patch)
        Hide
        Chuck Deal added a comment -

        oops... I had some jdk1.5 stuff in there...This is the same patch, but jdk1.4 compatible.

        Show
        Chuck Deal added a comment - oops... I had some jdk1.5 stuff in there...This is the same patch, but jdk1.4 compatible.
        Hide
        Chuck Deal added a comment -

        This is a simple quickstart to verify that this impl will work with both DateTime and Date objects with very little effort on the user's part.

        It is a standard 1.3 quickstart project minus the lib folder. You will also need to add joda-time as a dependency.

        Show
        Chuck Deal added a comment - This is a simple quickstart to verify that this impl will work with both DateTime and Date objects with very little effort on the user's part. It is a standard 1.3 quickstart project minus the lib folder. You will also need to add joda-time as a dependency.
        Hide
        Chuck Deal added a comment -

        This patch affects:
        DateTextField
        PatternDateConverter
        StyleDateConverter
        DateConverter (renamed to DateTimeConverter)

        Show
        Chuck Deal added a comment - This patch affects: DateTextField PatternDateConverter StyleDateConverter DateConverter (renamed to DateTimeConverter)

          People

          • Assignee:
            Juergen Donnerstag
            Reporter:
            Chuck Deal
          • Votes:
            7 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:

              Development