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

Fixing duplicate key warning when minDate or maxDate #131

Merged
merged 4 commits into from Sep 25, 2017
Merged

Fixing duplicate key warning when minDate or maxDate #131

merged 4 commits into from Sep 25, 2017

Conversation

monteiz
Copy link
Contributor

@monteiz monteiz commented Jul 17, 2017

Motivation and Context

If minDate or maxDate are set, several duplicate key warnings are raised. This commit fix that.
Moreover, the handleClick function binding has been moved out from the render method for potentially performance reasons:

We generally recommend binding in the constructor or using the property initializer syntax, to avoid this sort of performance problem.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

If minDate or maxDate are set, several duplicate key warnings are raised. This commit fix that.

Moreover, the function binding has been moved out from the render method for performance reasons.
@monteiz monteiz changed the title Fixing duplicate key warning and function binding Fixing duplicate key warning when minDate or maxDate Jul 17, 2017
@travellingprog
Copy link
Contributor

Thanks, I'm not the library maintainer but I came across this bug too and I'm glad you already fixed it in your branch. You should fix the indentation in your changes, though.

@travellingprog
Copy link
Contributor

travellingprog commented Aug 28, 2017

For people who want to remove this warning from their browser console, I made a fork of monteiz' branch where I ran the build process, to update the imported library.

With Yarn, you can use this build by running:
yarn add https://github.com/travellingprog/react-bootstrap-date-picker#fix-duplicate-key-warning

You should also be able to grab this branch with a regular NPM install, I believe by specifying a specific commit rather than the branch name, but i haven't tried it.

@monteiz
Copy link
Contributor Author

monteiz commented Aug 28, 2017

Hi @travellingprog,
please submit a pull request to my fork. In case @pushtell shows up again, he can quickly merge the fix and everybody is happy.

@travellingprog
Copy link
Contributor

@monteiz done

I also found and fixed another min/maxDate related bug in another branch (#137). And I have a 3rd branch that has both fixes (https://github.com/travellingprog/react-bootstrap-date-picker/tree/fix-makeDateValue-bug).

That's the branch I'm using in my app now, installed with

yarn add "https://github.com/travellingprog/react-bootstrap-date-picker#fix-makeDateValue-bug"

@JonShort JonShort self-requested a review September 25, 2017 09:33
@JonShort JonShort self-assigned this Sep 25, 2017
@JonShort
Copy link
Contributor

I've tested these changes locally, everything seems fine - the errors in the unit tests seem to be fixed by this also 👍

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.

None yet

3 participants