added reopenPickerOnClearDates prop

[agupta93]

This PR is in response to the issue #68 . As requested a prop reopenPickerOnClearDates has been added which governs whether the picker will be shown when clear is clicked.

By default the value is set to false.

[ljharb]

Naming the prop in this way makes it a breaking change - it'd be preferred to find a way to name the (default false) prop so that the reopen behavior is the default.

[agupta93]

@ljharb okay will do the same

[agupta93]

@ljharb PR updated please check

[ljharb]

curly braces need spaces inside them

[agupta93]

@ljharb made all the changes recommended by you please check

[ljharb]

Thanks, LGTM! It'd be great to also add an example for it in the storybook.

[agupta93]

@ljharb there is already a case for clear button would modify the same right according to the new feature added. One more thing is there a channel or IRC channel where the developers of this repo can be contacted for doubts in the code?

[agupta93]

added the example in storybook also

[majapw]

If you click the clear button now when the datepicker is open, I don't think this will actually hide the picker so this prop name is a misnomer. Something like dontReopenPickerOnClear maybe makes more sense... or even in this case, it might be worth it to have the semver breaking change of reopenPickerOnClear. What do you think @ljharb?

[ljharb]

If we're comfortable with a breaking change, then sure - that might be better.

[majapw]

I think that would be fine. We can couple the release with #53 for 3.0.0. I think, semantically, reopenPickerOnClearDates seems like a better name

[agupta93]

@ljharb and @majapw so is it final then the prop name would be reopenPickerOnClearDates whose default value would be false?

[majapw]

Thanks for the example. In addition to my comment about the naming, we'll definitely need to add a bit to the README about this prop.

We don't have a channel for discussing this code yet, but opening up an issue is probs the easiest way to get in touch.

[agupta93]

@majapw yaa i saw your comment, whatever name you guys finalize wont be a major change so once it finalized will change the name and add the corresponding readme regarding it. This would be my first commit to the library and would like to solve other open issues as well.

I asked for some channel so as to ficilitate some doubts that i might have in the code :)

[agupta93]

@majapw and @ljharb as discussed added a prop reopenPickerOnClearDates which defaults to false which means that no picker is shown by default on clear

Added a corresponding example in the storybook and also added the README for the same

[majapw]

Hi @agupta93! It looks like this is breaking the onFocusChange test now (which makes sense). If you could address that and maybe even add a test for this new prop, that would be awesome. :) We can merge it in after the tests change.

Can you also rebase locally and push it up instead of hitting the update branch button? That would help immensely in keeping our tree clean.

[agupta93]

@majapw corrected the build error and added the test for the same please check :)

[majapw]

So probably the structure of these tests should look something like:

describe('#clearDates', () => {
  describe('props.reopenPickerOnClearDates is truthy', () => {
    describe('props.onFocusChange', () => {
      it('is called once', () => {
        ...
      });

      it('is called with arg START_DATE', () => {
        ...
      });
  });

  describe('props.reopenPickerOnClearDates is falsey', () => {
    describe('props.onFocusChange is not called', () => {
      ...
    });
  });
});

Some of these tests are duplicates right now.

[majapw]

Right now the travis build is failing on some linting rules! You should address those and also run git rebase origin/master before you push back up to your remote branch. ✌️

[agupta93]

@majapw all changes done as asked hope everything looks fine now ✌️

[majapw]

LGTM. Is there any way you can squash your five commits into one single commit? I'll merge it in after that.

[agupta93]

@majapw commits squashed !!

[jamhall]

@agupta93 ! Thank you!