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

Fix Date Control Regression #1210

Merged
merged 1 commit into from
Mar 29, 2018
Merged

Fix Date Control Regression #1210

merged 1 commit into from
Mar 29, 2018

Conversation

tech4him1
Copy link
Contributor

@tech4him1 tech4him1 commented Mar 28, 2018

- Summary

Fix regression from b7fcb8b. Dates were disappearing completely until the field was clicked into.

- Test plan

Manually tested using datetime widget.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@verythorough
Copy link
Contributor

verythorough commented Mar 28, 2018

Deploy preview for netlify-cms-www ready!

Built with commit a294937

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

@verythorough
Copy link
Contributor

verythorough commented Mar 28, 2018

Deploy preview for cms-demo ready!

Built with commit a294937

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

@tech4him1 tech4him1 changed the title WIP: Fix Date Control Fix Date Control Mar 28, 2018
@tech4him1 tech4him1 requested review from talves and erquhart March 28, 2018 20:12
@tech4him1 tech4him1 changed the title Fix Date Control Fix Date Control Regression Mar 28, 2018
Copy link
Collaborator

@talves talves left a comment

Choose a reason for hiding this comment

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

Just checking 😄

this.format = field.get('format') || (includeTime ? DEFAULT_DATETIME_FORMAT : DEFAULT_DATE_FORMAT);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tech4him1 any reason in constructor rather than in componentWillMount

I would have to test, but I would think the format could change from one collection to the next right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The format can change between collections.

Can you explain when componentWillMount should be used versus constructor?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I may have made a bad assumption here. I thought we were talking about state changes 😛

Copy link
Collaborator

Choose a reason for hiding this comment

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

My assumption was incorrect, sorry for not looking closer first.

Copy link
Collaborator

@talves talves left a comment

Choose a reason for hiding this comment

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

Tested and works as expected.

this.format = field.get('format') || (includeTime ? DEFAULT_DATETIME_FORMAT : DEFAULT_DATE_FORMAT);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

My assumption was incorrect, sorry for not looking closer first.

Fix regression from b7fcb8b. Dates were disappearing completely until
the field was clicked into.
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.

4 participants