Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs] setDate triggerChange inverted behaviour #666

Closed
wants to merge 1 commit into from

Conversation

mlazze
Copy link

@mlazze mlazze commented Mar 8, 2017

File Reference

In docs it's implied that onChange events not firing is the default behaviour, but the default behaviour is to fire onChange event, unless false is passed as argument.

@RobbieTheWagner
Copy link
Contributor

RobbieTheWagner commented Mar 8, 2017

This seems to have been changed from previous versions https://github.com/chmln/flatpickr/blob/v2.3.2/src/flatpickr.js#L1259

I would argue that we should set the behavior back to the old way.

Why would we want an onChange fired when you set the initial value? Should only be when actually changed.

@chmln
Copy link
Member

chmln commented Mar 10, 2017

Funny enough, this exact issue bit me during plugin development, when a setDate() from an onChange hook caused stack overflow.

I think the previous value is more sensible.

@RobbieTheWagner
Copy link
Contributor

@chmln I would agree! Should I create an issue for it, or is tracking via this PR okay? Can probably close this PR, if we are going to set it back to the old way. No need to update the docs in that case.

@chmln
Copy link
Member

chmln commented Mar 10, 2017

@rwwagner90 I'll just close this PR as soon as I revert the behavior

@RobbieTheWagner
Copy link
Contributor

Thanks @chmln! Looking forward to the next release, so I can close out all the ember-flatpickr issues we encountered from this 😄

@RobbieTheWagner
Copy link
Contributor

@chmln any chance we could get a release to patch this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants