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

Adds ability to use a custom date format #46

Closed
wants to merge 2 commits into from

Conversation

meg2208
Copy link

@meg2208 meg2208 commented Nov 6, 2014

A date format string can be entered as the prop dateFormat to the DatePicker element. If you do not enter this prop, it will default to YYYY-MM-DD, which was previously the only valid format allowed. The requirements of one of my current projects was that I needed a different format, so I put in the issue #45. Please let me know if you have any questions. Thanks!

@@ -82,6 +82,7 @@ var DatePicker = React.createClass({
<div>
<DateInput
date={this.props.selected}
dateFormat={this.props.dateFormat || 'YYYY-MM-DD'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's default to YYYY-MM-DD by setting it in getDefaultProps.

@RSO
Copy link
Contributor

RSO commented Nov 6, 2014

Awesome! Thanks for submitting this. I have two minor comments, after that I think this is good to go.

@meg2208
Copy link
Author

meg2208 commented Nov 7, 2014

@RSO no problem! This should be ready to merge.

@RSO
Copy link
Contributor

RSO commented Nov 11, 2014

Sorry for getting back to you so late, but I've just discovered a problem with this pull request.

A while ago I added a functionality that would increment/decrement the part of the date which your cursor highlighted. This no longer works properly when you change the dateFormat, because of all the implicit dependencies on the dateFormat..

The way I see it there are two things we could do here:

  1. Change the behaviour so that the up/down keys always just increment the day
  2. Add a method getDatePartForKeyPosition to the props

I'd vote for 1. What do you think?

@@ -254,6 +254,12 @@ var DatePicker = React.createClass({displayName: 'DatePicker',
};
},

getDefaultProps: function() {
return {
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation level on these lines are 4 spaces instead of 2.

@meg2208
Copy link
Author

meg2208 commented Nov 12, 2014

I will try to dig in as soon as I have a chance. Probably within the next 5 days. This is a valid concern and a nice feature of the plugin. After a quick glance at the code, I think we should be able to re-parse out the year/month/day based on the keydown event target and retain the current functionality...since the date format string is available as a prop. I think what I just described is pretty much #2. Anyways, I think I would have a go at that before resorting to removing some of the value-add functionality and implementing #1.

Thanks for the input!

@martijnrusschen
Copy link
Member

Closing this since #48 is already merged.

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.

None yet

3 participants