-
-
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
return date object from date/datetime widgets if no format set #1296
Conversation
@siliconchild please test and confirm this works for you. |
Deploy preview for cms-demo ready! Built with commit 21cfa71 |
Deploy preview for netlify-cms-www ready! Built with commit 21cfa71 |
081c595
to
84abbc0
Compare
CHANGELOG.md
Outdated
@@ -4,6 +4,8 @@ | |||
Changes that have landed in master but are not yet released. | |||
Click to see more. | |||
</summary> | |||
|
|||
* BREAKING: return date object from date/datetime widgets if no format set ([@erquhart](https://github.com/erquhart) in [#1296](https://github.com/netlify/netlify-cms/pull/1296)) |
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.
Maybe it would be better to say "Possibly Breaking", since we're not doing a major release.
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.
Yeah, agreed
onChange(formattedValue); | ||
} else { | ||
onChange(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.
Is it better to call datetime.toDate()
here so that the formatters have a JS date object to work with instead of a Moment object?
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.
Nevermind, this may not work well for JSON: https://momentjs.com/docs/#/displaying/as-json/
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.
Yeah it matches previous behavior.
@@ -67,7 +79,7 @@ export default class DateControl extends React.Component { | |||
|
|||
render() { | |||
const { includeTime, value, classNameWrapper, setActiveStyle, setInactiveStyle } = this.props; | |||
const format = this.format; | |||
const format = this.formatDisplay; |
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.
Not sure if we need this since it doesn't actually affect the display, just the parsing. I haven't tested, though.
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.
Agreed, updated.
Reworked to consistently return a raw date rather than a moment object when no format is configured - the difference was manifesting in the preview pane, raw date stringifies with the timezone code in parens, moment does not. |
This is ready for review. |
We'll leave this in 2.0 since we're getting close anyway. |
BREAKING CHANGE As of 1.0, the documented behavior for the date and datetime widgets was to always return a string value, but they were instead returning a date object if the default date was not manually changed by the user. This was addressed in #1143, but it became clear afterward that static site generators were depending on the raw date objects that Netlify CMS was unintentionally producing. Remaining as is or addressing the bug were both "breaking" states, so this commit reverts to producing raw date objects when no format is explicitly set. It is now considered an edge case to require string dates, as most static site generators expect to parse a raw date against formatting in a site's templates. Also note that this commit improves the original behavior by always providing a date object when no format is provided, even if the user manually changes the value.
Hi, I'm currently looking at this page and I'm seeing that this PR is included, I tried re-adding the CMS to my site to confirm this but the date still has quotes when creating new items. Am I misunderstanding/misreading the details in the page I linked? |
Right, the page shows what's on Also note that comparisons between 1.x releases and master won't work because 1.x releases are coming from the Or you can just check release notes, we keep them accurate. |
@erquhart Thanks |
…org#1296) * return date object from date/datetime widgets if no format set BREAKING CHANGE As of 1.0, the documented behavior for the date and datetime widgets was to always return a string value, but they were instead returning a date object if the default date was not manually changed by the user. This was addressed in decaporg#1143, but it became clear afterward that static site generators were depending on the raw date objects that Netlify CMS was unintentionally producing. Remaining as is or addressing the bug were both "breaking" states, so this commit reverts to producing raw date objects when no format is explicitly set. It is now considered an edge case to require string dates, as most static site generators expect to parse a raw date against formatting in a site's templates. Also note that this commit improves the original behavior by always providing a date object when no format is provided, even if the user manually changes the value. * produce raw date when no format is provided
…org#1296) * return date object from date/datetime widgets if no format set BREAKING CHANGE As of 1.0, the documented behavior for the date and datetime widgets was to always return a string value, but they were instead returning a date object if the default date was not manually changed by the user. This was addressed in decaporg#1143, but it became clear afterward that static site generators were depending on the raw date objects that Netlify CMS was unintentionally producing. Remaining as is or addressing the bug were both "breaking" states, so this commit reverts to producing raw date objects when no format is explicitly set. It is now considered an edge case to require string dates, as most static site generators expect to parse a raw date against formatting in a site's templates. Also note that this commit improves the original behavior by always providing a date object when no format is provided, even if the user manually changes the value. * produce raw date when no format is provided
BREAKING CHANGE
As of 1.0, the documented behavior for the date and datetime widgets was
to always return a string value, but they were instead returning a date
object if the default date was not manually changed by the user. This
was addressed in #1143, but it became clear afterward that static site
generators were depending on the raw date objects that Netlify CMS was
unintentionally producing. Remaining as is or addressing the bug were
both "breaking" states, so this commit reverts to producing raw date
objects when no format is explicitly set.
It is now considered an edge case to require string dates, as most
static site generators expect to parse a raw date against formatting in
a site's templates.
Also note that this commit improves the original behavior by always
providing a date object when no format is provided, even if the user
manually changes the value.
Fixes #1271.
- Description for the changelog
return date object from date/datetime widgets if no format set