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

Custom parsing for dates #653

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tnolli
Copy link

@tnolli tnolli commented Jan 8, 2019

This is a possible fix for #597

@coveralls
Copy link

coveralls commented Jan 8, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 1cb2670 on tnolli:features/typed-dates-custom-parsing into 7cc1c32 on charliekassel:master.

Copy link

@sylann sylann left a comment

Choose a reason for hiding this comment

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

Great solution!

I've searched for javascript string parsing solutions that could work with a format string, but it can become tricky.

I think this solution should be merged quite rapidly since users who aren't using US dates, currently face a major bug with the typeable datepicker feature.
Allowing developers to use their own parser, should cover most of the cases, especially when predictable date formats are used.

👍

@@ -118,10 +119,11 @@ export default {
}

if (this.typeable) {
const typedDate = Date.parse(this.input.value)
const fn = this.parse || Date.parse
const typedDate = fn(this.input.value)
Copy link

Choose a reason for hiding this comment

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

I suggest your replace line 122-123 with this:

const parsed = this.parse(this.input.value)

if (!isNaN(typedDate)) {
this.typedDate = this.input.value
this.$emit('typedDate', new Date(this.typedDate))
this.$emit('typedDate', new Date(typedDate))
Copy link

Choose a reason for hiding this comment

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

I suggest your replace line 126 with this:

this.$emit('typedDate', parsed)

@@ -130,7 +132,8 @@ export default {
* called once the input is blurred
*/
inputBlurred () {
if (this.typeable && isNaN(Date.parse(this.input.value))) {
const fn = this.parse || Date.parse
if (this.typeable && isNaN(fn(this.input.value))) {
Copy link

Choose a reason for hiding this comment

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

I suggest your replace line 135-136 with this:

if (this.typeable && isNaN(this.parse(this.input.value))) {

@@ -118,6 +119,7 @@ export default {
type: [String, Function],
default: 'dd MMM yyyy'
},
parse: Function,
Copy link

Choose a reason for hiding this comment

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

I suggest you replace line 122 with this:

parse: {
  type: Function,
  default: v => new Date(v),
}

@tnolli
Copy link
Author

tnolli commented Jan 27, 2019

@SylannBin thanks for the code review; I applied your suggestions (almost entirelly, Ijust moved the default to DateInput instead of Datepicker),

@zzen
Copy link

zzen commented Apr 10, 2019

OK I just spent an hour debugging this, only to arrive at the same solution (we need a custom parsing function). Any chances this can get merged (or reasons not to)?

@lechinoix
Copy link

Hello ! Also facing this problem using this lib, is it possible to merge ?

@aesfer
Copy link

aesfer commented Apr 17, 2019

This feature not only solves a bunch of potential parsing issues but it also enables new use cases out of the box, such as:

  • Triggering validation errors (which occur during the parse function).
  • Contextual date parsing. For instance in a fromDate - toDate range, I would like 1980 to be parsed as 1980/01/01 in fromDate and parsed as 1908/12/31 in toDate.

Hopefully this gets merged soon.

@ConorSheehan1
Copy link

ConorSheehan1 commented Oct 21, 2019

First of all, thanks @charliekassel for all your work on this. It's awesome!

I think this pr could close a huge amount of issues. Just for example:

The equivalent pr for delegating formatting dates was merged the same day it was opened!
#232 This has been open for 9 months now, and the last comment was 6 months ago.

Also, I would choose this over #536 because it lets users choose how the date is parsed, rather than changing the string that get's passed to Date.parse.

I just read #679 (comment) and saw you haven't had time to work on this, and are considering co-maintainers, which I think is a great idea!
Just wondering, is there any chance of this being merged soon?

@otaviogaiao
Copy link

Any news on this?

@roeb
Copy link

roeb commented Dec 30, 2019

When is this feature merged? It is urgently needed for different projects.

@LeonidasJP
Copy link

Any updates on when/whether this PR will be merged? It blocks development in several projects at the moment, so an update would be very nice ;-)
Does @tnolli have to rebase his branch first, before this can be merged?

@soulaimanassikiou
Copy link

Here! also waiting for the merge!!!

@fabio-fafle
Copy link

Any news?

@heivo
Copy link

heivo commented Jul 6, 2020

Is there anything that we can do to help this getting merged?

kunalpawar7788 pushed a commit to kunalpawar7788/milife-front that referenced this pull request Nov 4, 2021
when this gets merged, we should be able to make things work.
charliekassel/vuejs-datepicker#653
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