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

exclude time from default date widget format #1143

Merged
merged 2 commits into from
Mar 4, 2018
Merged

exclude time from default date widget format #1143

merged 2 commits into from
Mar 4, 2018

Conversation

erquhart
Copy link
Contributor

Sets correct and documented format for Date widget (YYYY-MM-DD).

Closes #1119.

@verythorough
Copy link
Contributor

verythorough commented Feb 28, 2018

Deploy preview for netlify-cms-www ready!

Built with commit cc30539

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

@verythorough
Copy link
Contributor

verythorough commented Feb 28, 2018

Deploy preview for cms-demo ready!

Built with commit cc30539

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

@@ -42,7 +42,7 @@ export default class DateControl extends React.Component {

render() {
const { includeTime, value, classNameWrapper, setActiveStyle, setInactiveStyle } = this.props;
const format = this.format || moment.defaultFormat;
const format = this.format || moment[includeTime ? 'defaultFormat' : 'HTML5_FMT.DATE'];
Copy link
Contributor

Choose a reason for hiding this comment

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

'HTML5_FMT.DATE' won't work as it's not a key name, since DATE is a property of HTML5_FMT.

Copy link
Contributor

Choose a reason for hiding this comment

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

const format = this.format || (includeTime ? moment.defaultFormat : moment.HTML5_FMT.DATE);

should work (not tested)

Copy link
Contributor Author

@erquhart erquhart Mar 2, 2018

Choose a reason for hiding this comment

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

Good catch! 🤦‍♂️

@tech4him1
Copy link
Contributor

I'm assuming this shouldn't be classified as a breaking change if we're just changing it to match our docs? It is changing the output of pre-existing entries, though.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 2, 2018

Right not, breaking, just a bugfix. This is the documented behavior and we weren't doing it.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 2, 2018

@tech4him1 fixed.

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

Code looks great! However, HTML5_FMT.DATE wasn't added until Moment 2.20.0, and we're on 2.11.2, so that will need updated as well.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 2, 2018

Drat.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 2, 2018

@tech4him1 fixed 4 realz.

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

LGTM. I take it you don't want to upgrade Moment. :)

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

Just noticed that our value in Redux is still the ISO format, not the actual formatted value that we want. The file that is output has the correct format, but the preview will not. Thoughts?

@tech4him1 tech4him1 changed the title exlude time from default date widget format exclude time from default date widget format Mar 2, 2018
@tech4him1
Copy link
Contributor

@erquhart I went ahead and pushed a commit to save the formatted value in Redux, feel free to revert if you don't like it. 😄

@erquhart
Copy link
Contributor Author

erquhart commented Mar 4, 2018

@tech4him1 much better place to do it, thank you.

@erquhart erquhart merged commit b7fcb8b into master Mar 4, 2018
@erquhart erquhart deleted the date-format branch March 4, 2018 03:45
erquhart added a commit that referenced this pull request 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.
erquhart added a commit that referenced this pull request 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.
erquhart added a commit that referenced this pull request 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.
erquhart added a commit that referenced this pull request 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.
erquhart added a commit that referenced this pull request May 25, 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.
erquhart added a commit that referenced this pull request May 25, 2018
* 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 #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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants