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

return date object from date/datetime widgets if no format set #1296

Merged
merged 2 commits into from
May 25, 2018

Conversation

erquhart
Copy link
Contributor

@erquhart erquhart commented Apr 20, 2018

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

@erquhart
Copy link
Contributor Author

@siliconchild please test and confirm this works for you.

@verythorough
Copy link
Contributor

verythorough commented Apr 20, 2018

Deploy preview for cms-demo ready!

Built with commit 21cfa71

https://deploy-preview-1296--cms-demo.netlify.com

@verythorough
Copy link
Contributor

verythorough commented Apr 20, 2018

Deploy preview for netlify-cms-www ready!

Built with commit 21cfa71

https://deploy-preview-1296--netlify-cms-www.netlify.com

@erquhart erquhart force-pushed the date-object branch 2 times, most recently from 081c595 to 84abbc0 Compare April 20, 2018 17:05
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))
Copy link
Contributor

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.

Copy link
Contributor Author

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);
Copy link
Contributor

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?

Copy link
Contributor

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/

Copy link
Contributor Author

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;
Copy link
Contributor

@tech4him1 tech4him1 Apr 20, 2018

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, updated.

@erquhart
Copy link
Contributor Author

erquhart commented Apr 24, 2018

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.

@erquhart
Copy link
Contributor Author

This is ready for review.

@erquhart
Copy link
Contributor Author

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.
@erquhart erquhart merged commit 9fd0ff4 into master May 25, 2018
@erquhart erquhart deleted the date-object branch May 25, 2018 16:03
@ghost
Copy link

ghost commented May 25, 2018

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?

@erquhart
Copy link
Contributor Author

erquhart commented May 25, 2018

Right, the page shows what's on master that is not in 1.8.4. Because that PR is a breaking change, we're saving it for 2.0. Pre-releases of 2.0 will begin in the coming weeks.

Also note that comparisons between 1.x releases and master won't work because 1.x releases are coming from the v1 branch, which receives cherry picked commits from master, so every commit in master moving forward will seem to be missing from future 1.x releases. Instead, you can check at the bottom of a PR for a commit occurring after merge - those happen when the merged pull request is backported (cherry picked) to v1.

Or you can just check release notes, we keep them accurate.

@ghost
Copy link

ghost commented May 25, 2018

@erquhart Thanks

brianlmacdonald pushed a commit to brianlmacdonald/netlify-cms that referenced this pull request May 29, 2018
…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
brianlmacdonald pushed a commit to brianlmacdonald/netlify-cms that referenced this pull request May 29, 2018
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datetime widget is wrapping date in single quotes
3 participants