Skip to content

Conversation

@prsdthkr
Copy link
Member

Description

Currently this.props.onChange is called from multiple places without checking if the formattedDate in the state has truly changed. This leads to this.props.onChange being called with the same payload multiple times. To prevent this we should explicitly check if formattedDate has changed before calling this.props.onChange with the new payload.

@prsdthkr prsdthkr requested review from a team, jacobdevera and meganaconley April 23, 2020 00:49
@netlify
Copy link

netlify bot commented Apr 23, 2020

Deploy preview for fundamental-react ready!

Built with commit 9b1f1f8

https://deploy-preview-973--fundamental-react.netlify.app

@jacobdevera
Copy link
Contributor

Not to say this change is wrong, but I'm having a hard time reproducing it. I've been logging, setting breakpoints, etc. but any onChange function I set or any of the related functions (updateDate, resetState, validateDates) only seem to be called once whether I'm typing in dates or selecting a date with the calendar.

@prsdthkr
Copy link
Member Author

Not to say this change is wrong, but I'm having a hard time reproducing it. I've been logging, setting breakpoints, etc. but any onChange function I set or any of the related functions (updateDate, resetState, validateDates) only seem to be called once whether I'm typing in dates or selecting a date with the calendar.

Oops! forgot to add before/after GIF. Added.

For example, onChange is fired after onBlur with the same data, or even after selecting the same date again.

@jacobdevera
Copy link
Contributor

Okay, I can see those cases, that makes sense to prevent.

@prsdthkr prsdthkr self-assigned this Apr 23, 2020
@prsdthkr prsdthkr added the bug Something isn't working / Issues in the code label Apr 23, 2020
@prsdthkr prsdthkr merged commit 7eb510b into master Apr 23, 2020
@prsdthkr prsdthkr deleted the fix/datepicker-onchange branch July 21, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working / Issues in the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants