Allow multiple input formats, add onClear callback for better Ember/Knockout/etc integration and allow other date libraries instead of Moment.js

[bjbrewster]

Thanks for creating this awesome and yet simple date picker. We are using this library on a fundraising portal raising money for charity so know that its helping make a difference in the world. We needed a few changes to make it easier to integrate with Ember.js so I would like to contribute back to your project.

This change is loosely based on #57 to add callback on clear/invalid date and also fixes #61, #48, #15, #93, #98. I decided upon onClear for the name of the callback, in the same vein as onSelect, named after the method clearDate() that I extracted from setDate(). Feel free to suggest better names. It was very tempting to call onSelect callback with null for empty/invalid date, but this would most likely break a lot of code by the Pikaday community that relies on the callback always having a valid date so a new callback must be introduced instead. I would also like to add a suite of automated JS tests to improve code quality.

• Cleaned up setDate() so both null date and invalid date string clear the current selection.
• Extracted clearDate() method from setDate() for clearing the current selection with options for clearing the input field (if set) and preventing onClear callback.
• onClear callback is called by clearDate() so users can be notified of invalid/empty date. The user's callback can get the field text value to show a required validation message if empty or invalid date validation message if not empty.
• onSelect and onClear are useful when integrating with MVC frameworks such as Knockout.js and Ember.js where onSelect and onClear sets and clears the binding/component's date value.
• Added examples for Knockout (knockout.js integration #48) and Ember.
• clearInvalidInput option can be set to true to automatically clear the input field value (if field is set) on invalid input. This is a simple way of telling the user the entered date is invalid.
• parseDate() method was extracted from _onInputChange() to remove duplicate date string parsing in Pikaday constructor for defaultDate and in setDate() (was only parsing with JS Date.parse?).
• parseDate() method was added to Pikaday prototype instead of a helper/private method to allow for overriding both toString() and parseDate() on Pikaday.prototype to support other JS date libraries.
  See Datejs and Sugar examples.
• Fixed issue Set blank default date #98 with Datejs where Date.parse() returns null instead of NaN for invalid input which caused a default date of 1/1/1970 in empty input fields.
• A big feature of Moment.js is that it allows for multiple input formats. inputFormats option was added to allow users to specify an array of allowed input formats. Defaults to the display format. Moment example updated accordingly.
• Added fix for issue wrong month shown when setting minDate to a later year #93 where wrong month is sometimes shown when minDate's year > default selected date. There's a separate issue where setMinDate does not update the internal min/maxMonth and min/maxYear used in draw method.
• README updated accordingly (including defaultDate expected type Format of defaultDate #15)

[bjbrewster]

Sorry, I hate seeing nested ternary operators on a single line.

[bjbrewster]

I was also thinking of parseFormats for this option name as it is used by parseDate() and seemed more explicit. Feel free to suggest better names for any of these option and method names etc.

[bjbrewster]

I didn't think this section belonged under the Methods heading

[bjbrewster]

Is there a reason this is checking if min/maxMonth isNan? As far as I can tell, the config function checks the min and max dates are valid dates before extracting the min and max months. I think minMonth and maxMonth should be defaulted to 0 and 11 instead of undefined, however.

[rikkert]

Spaces on the eol are significant in markdown.

[bjbrewster]

Thanks, I wasn't sure about this. I'll fixup the README accordingly. Any other feedback? :-)

[rikkert]

Do you mind rebasing from master?

[bjbrewster]

Sure. I'm almost done adding some Mocha tests too.

On Tue, Apr 8, 2014 at 4:23 PM, Ramiro Rikkert notifications@github.comwrote:

Do you mind rebasing from master?

Reply to this email directly or view it on GitHubhttps://github.com//pull/134#issuecomment-39815402
.

[bjbrewster]

I've just come back from holidays so I'll get back on top of this PR and hope to have it done soon. I'll put up an early example of the test framework I've made to see if it's on the right track.

[rikkert]

I have a test suite ruining trough testing.
Could you already push your tests on your fork so I could move these in already?
Thanks!

[bjbrewster]

I used Mocha as you asked and created a tests folder with a pikaday.html file with the following:

<html>
<head>
  <meta charset="utf-8">
  <title>Pikaday Tests</title>
  <link rel="stylesheet" href="../node_modules/mocha/mocha.css" />
</head>
<body>
  <div id="mocha"></div>
  <script src="../node_modules/mocha/mocha.js"></script>
  <script src="../node_modules/should/should.js"></script>
  <script src="../pikaday.js"></script>
  <script>
    mocha.setup('bdd')

    describe('Pikaday', function() {
      describe('#toString()', function() {
        it('should return empty string when date not set', function() {
          var pikaday = new Pikaday();
          pikaday.toString().should.equal('');
        })

        it('should return date string when date is set', function() {
          var date = new Date('2014-04-25');
          var pikaday = new Pikaday();
          pikaday.setDate(date);
          pikaday.toString().should.equal(date.toDateString());
        })
      })

      describe('#getDate()', function() {
        it('', function() {
        })
      })

      describe('#setDate()', function() {
        it('', function() {
        })
      })

      describe('#clearDate()', function() {
        it('', function() {
        })
      })

      describe('#parseDate()', function() {
        it('', function() {
        })
      })

      describe('#gotoDate()', function() {
        it('', function() {
        })
      })

      ...
    })

    mocha.checkLeaks();
    mocha.globals(['jQuery']);
    mocha.run();
  </script>
</body>
</html>

I wasn't sure how to have an html test file that uses Moment and one without Moment. Including pikaday.js automatically includes Moment.js as it finds the exports interface from Node.js (as used by Mocha) and auto loads it. So I was going to flesh out this html test suite assuming Moment.js was available. I couldn't find any other way of running client-side feature tests using Mocha. Writing standard js unit tests with Mocha and run at the command line complain about missing objects and methods such as window.addEventListener.

Was this the kind of test suite and setup you had in mind with Mocha?

[rikkert]

Check out the test framework and test stubs that I added.
They run using mocha / expect.js / browserify / testling.
Is it possible to split out this PR to smaller ones that include tests and docs?

[bjbrewster]

Sure, i can do that. Am busy with work this week so will continue on the
weekend. Thanks for adding the test stubs, that will make my life easier.

On Thursday, 1 May 2014, Ramiro Rikkert notifications@github.com wrote:

Check out the test framework and test stubs that I added.
They run using mocha / expect.js / browserify / testling.
Is it possible to split out this PR to smaller ones that include tests and
docs?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/134#issuecomment-41831870
.

[bjbrewster]

@rikkert How do I create multiple PRs for Pikaday from my bjbrewster/Pikaday fork? I guess I could fork Pikaday multiple times or scale back my single PR to a smaller set of fixes and once you've merged that I can create another PR etc?

[rikkert]

Easiest is to make a new branch for each PR of the relevant commit.
After you can create a new PR using that branch.
Would be great to get this ball rolling again.

[mhkeller]

Have these been merged somewhere or is this still in the works? I would love to start using some of these features.

[bjbrewster]

Hello,

I got a bit lost regarding creating many PRs from my own repo. I'll try to
get the ball rolling again and see this through.

Cheers,
Bren

On Fri, Sep 12, 2014 at 12:00 PM, Michael Keller notifications@github.com
wrote:

Have these been merged somewhere or is this still in the works? I would
love to start using some of these features.

—
Reply to this email directly or view it on GitHub
#134 (comment).

[mhkeller]

Great! In the meantime I'm just using your fork.

I have one suggestion: when you call clearDate in my use case it's also be desirable to clear the min and max date. Perhaps that could be integrated into that method.

EDITED to remove the question on a proper way to clear min and max time since I just figured it out.

[bjbrewster]

Not sure if clearDate() should also clear the min and max dates. That should be separate method(s) in my opinion. clearDate() is more for clearing the selected date, which shouldn't affect the min/max validation.

[mhkeller]

Ya it might not be for everybody. I'm setting the min/max more dynamically based on what the user has filled into the other box as opposed to hard limits for the page's state. Perhaps it's an argument the same way you have a boolean for clearing the display. But, again, if it's a small use case, setting it in the callback is easy enough.

[drewcovi]

So is this PR dead?

[mhkeller]

I still think it's an improvement.

[mazing]

Great PR. Why isn't it accepted?

[jaconi]

I find this PR very useful. Any chances it will be accepted?

[gliesche]

+1

[foohooboo]

@bjbrewster @mhkeller Is this PR dead? Is it safe to replace into my existing code that is already using the current pikaday? The functionality I'm wanting is the multiple input formats.

[rikkert]

Yes, use the parse and toString method that have been added.