DatePicker: isRequired validation bug

[fantua]

Bug Report

• Package version(s): 0.88

Describe the issue:

Create form with TextField and DatePicker components. TextField is controlled component (by value and onChanged), so when typing some data to him - React will re-render form.

Actual behavior:

After re-render validation is fired.

Expected behavior:

Validation should fired only when DatePicker loses focus.

Additional question:

How check is DatePicker valid when form is submitting?
Perhaps better will be also add required prop to DatePicker input element this will prevent form submitting when DatePicker is empty?

[gitjain]

@fantua
I tried to repro the issue and seems TextField validates the date at submission only.
Can you share a codepen and more details if you are still facing the issue.

[fantua]

@gitjain http://codepen.io/fant/pen/bqzBZd enter something into the input.

[Jahnp]

@fantua, we're cleaning up old issues that haven't had traction on them in awhile--please feel free to reopen if this is still an issue.

@johannao76, since your team owns DatePicker, can you take a look at this and see if this still needs to be addressed?

[reewen]

I still have this issue with the latest version of office-ui-fabric-react (5.45.0). Could you guys open this issue again? Thank you! @fantua @Jahnp

[Jahnp]

Sure! @johannao76, any chance you or someone from your team could take a look at this when you have a chance?

[lorejoh12]

taking a look

[lorejoh12]

It looks like since we allow DatePicker to have the value passed as a property, there was some additional logic added to make sure that validation was run during componentWillReceiveProps. However, that fires every rerender (after initial render), and validation was running even though none of the props had actually changed. I added a quick check at the top to make sure the validation doesn't run unless props actually changed, and that appears to fix this issue.

[reewen]

@lorejoh12 Thanks for looking at this. But this behavior is still different from other components, like Dropdown and TextField. Rerendering doesn't fire other components(with required=true, and they also accept value passed as a property), so is this inconsistent? Can we make them consistent?

[lorejoh12]

Hmm, I don't see any actual validation in Dropdown, maybe you were referring to another component? As for TextField, that actually does seem to be what it's doing, although a little more cleanly:

Rather than comparing all props to see if they've change from past props, TextField just compares the new prop "value" to the state's value to see if it's changed before running any logic/validation, but the idea is the same.

Is it consistent enough here if we do that same check? Check to see if value is different than the state value?

[reewen]

Hmm, I find that TextField NEVER fires even if 1) the value change from non-empty to empty, 2) focus in the field and blur without entering text. TextField only fires when there is error message.
Can the DatePicker go with the same way?

[lorejoh12]

I'm not sure I understand what you're referring to by "fires". By fires, I'm referring to the validate method ever being called. TextField certainly fires "componentWillReceiveProps", which checks if value changed and fires delayedValidate, which then fires validate method. This is only one of the ways in which validate gets fired, but it's the one that's broken in DatePicker right now.

DatePicker currently fires validate without checking if the value actually changed. My updated PR will check that one of the properties we care about changed now before doing any validation, which means we'll get consistent behavior with TextField once this goes in.

[reewen]

Here, we mean the boarder changes to red.