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

Adding check to DatePicker componentWillReceiveProps to prevent unnecessary validation failures #3843

Merged
merged 4 commits into from
Jan 31, 2018

Conversation

lorejoh12
Copy link
Contributor

@lorejoh12 lorejoh12 commented Jan 30, 2018

Pull request checklist

Description of changes

DatePicker allows the consumer to pass a prop for the value, and a prop for isRequired. If a consumer changes the properties, it's possible that the required validation step might no longer be passing, so DatePicker was running validation during WillReceiveProps. However, it ran this regardless of whether the props actually changed, so the parent rerendering was causing validation to fail even without ever focusing the component. Here we add a quick check to make sure the serializable props actually changed before running that validation.

Focus areas to test

(optional)

@lorejoh12 lorejoh12 requested a review from johannao76 as a code owner January 30, 2018 23:55
@@ -142,6 +142,11 @@ export class DatePicker extends BaseComponent<IDatePickerProps, IDatePickerState
}

public componentWillReceiveProps(nextProps: IDatePickerProps) {
if (JSON.stringify(this.props) === JSON.stringify(nextProps)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to stringify before comparing?

@johannao76 johannao76 merged commit 7df5be2 into microsoft:master Jan 31, 2018
@lorejoh12 lorejoh12 deleted the jolore/datePickerValidationFix branch April 11, 2018 19:07
chrismohr pushed a commit to chrismohr/office-ui-fabric-react that referenced this pull request Apr 17, 2018
…essary validation failures (microsoft#3843)

* direct comparison of objects returns false, need to stringify to compare

* adding change file

* build didn't like ==, changing to ===

* final fixes
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DatePicker: isRequired validation bug
2 participants