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

<DateInput /> Don't real-time format the value when onChange #292

Closed
pololee opened this issue Sep 1, 2017 · 3 comments
Closed

<DateInput /> Don't real-time format the value when onChange #292

pololee opened this issue Sep 1, 2017 · 3 comments

Comments

@pololee
Copy link
Contributor

pololee commented Sep 1, 2017

autoformatting_date

As you can see in the gif, when you change a number in the middle of the value, it will auto format the value.

Proposal:

  1. We currently pass value and isValid to this.props.onChange here. We could pass another params to tell where the input comes from, inputField or calendar
  2. Add a new option realtimeFormatValue. Default true as to make it backward-compatible.
  3. When realtimeFormatValue is false, format the value onBlur (i.e. when lose focus.) We want to wait user to finish typing before trying to format the value.
@gthomas-appfolio
Copy link
Contributor

The issue is valid, but the proposal to format only on blur might be difficult and/or incompatible with a controlled component (where the value prop is updated with new Date).

In controlled environment, the onChange is triggered after every keystroke and the value prop is updated - if it's a valid Date it will be formatted causing the text field to be updated unexpectedly (bad). But if you wanted to update the Date from outside the component (for example, switching to a new data record) you'd expect the new Date to be formatted.

It seems that the behavior desired is closer to the uncontrolled component (where the defaultValue prop is set, and onChange is called with Date and whether it's valid. However, there is a secondary issue where Dates with days < 1 and > 31 are parsed as real Dates when doing inline editing with 0 prefixes.

@gthomas-appfolio
Copy link
Contributor

Paulus came up with a clever solution to use the input text similar to an uncontrolled component, and only update it on blur or prop update where the dates differ: #293
It may need a few more updates but will review with usages today.

@gthomas-appfolio
Copy link
Contributor

This should be resolved in #293 , and v1.29.0+ Thanks for finding

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

No branches or pull requests

2 participants