-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Allow typing in date widget #1247
Conversation
Deploy preview for cms-demo ready! Built with commit 532b311 |
Deploy preview for netlify-cms-www ready! Built with commit 532b311 |
@@ -36,9 +36,8 @@ export default class DateControl extends React.Component { | |||
|
|||
handleChange = datetime => { | |||
const { onChange } = this.props; | |||
if (!this.format || datetime === '') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do still need to handle the empty case here, in case the user wants to empty the field.
This looks great -- I think reverting to the previous value if the date is invalid is reasonable. Do you think we need to warn the user at that point, or just revert it? For simple changes, the user wouldn't necessarily notice that it was reverted (e.g. entered |
@tech4him1 The empty case is now supported and the user is warned onBlur that the date is invalid. |
This is definitely an improvement, thanks @Dammmien! I do find the warning jarring, especially considering the case where a user types in a date, and it's almost guaranteed to be the wrong format, then they get a generic warning and the field goes blank. I'd like us to consider attempting to Thoughts? |
Also, if we do choose to use |
@erquhart , @tech4him1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, works really well. Thanks @Dammmien!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks great, just one quick change.
@@ -34,16 +34,35 @@ export default class DateControl extends React.Component { | |||
} | |||
} | |||
|
|||
// Date is valid if datetime is a moment or Date object otherwise it's a string. | |||
// Handle the empty case, if the user wants to empty the field. | |||
isValid = datetime => (moment.isMoment(datetime) || datetime instanceof Date || datetime === ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isValid
has a specific use for widget controls, which we're not trying to use here, so can we change this to something else, maybe isValidDate
?
@erquhart Just so I understand -- we're not warning users if the date is invalid because of why? Now that we are parsing with Moment it should rarely be invalid. |
Right, so you'd have to type something pretty odd to get a blank field. The warning was a tad jarring anyhow, I don't think anyone will be thrown by this behavior. What do you think? |
Is there any reason we can't throw the warning only if the value can't be parsed by Moment, since that should be rare now? |
It is clearer that way - your call, I'm good with either. Sent with GitHawk |
@tech4him1 I changed |
* Allow typing in date widget * Handle empty case & warn user when invalid * Try parsing date with moment * Rename isValid -> isValidDate * Warn user when date is invalid
Fixes #1178 : Allow typing in date widget
- Summary
Allow typing in date widget by setting the date only if the format is valid
- Test plan
Test date widget behavior is ok
- Description for the changelog
Allow typing in date widget