Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix(dateparser): baseDate only care about dates #4767

Closed

Conversation

Foxandxss
Copy link
Contributor

So, setFullYear won't respect the time part, and since we want to respect the timezone, it doesn't make sense anyway right?

So I removed all the time part and also added tests to it.

Not entirely sure if I am right, but I think so.

Thoughts @wesleycho ?

@wesleycho
Copy link
Contributor

So we want to preserve the other information if possible because some people will use the same model with the timepicker & datepicker - there was a previous issue here involving the datepicker incorrectly modifying the other values.

@Foxandxss
Copy link
Contributor Author

So, if I do:

parse('1', 'd', new Date(2010, 10, 10, 23, 13));

I want to change the day to day 1, but maintain that hour (23:13), right?

If I do that (with this PR code), I get:

Mon Nov 01 2010 23:13:00 GMT+0100 (CET)

Is that right?

@Foxandxss
Copy link
Contributor Author

In fact, this PR is not doing anything different, the other 4 parameters are getting ignored, so I removed them, removed them from initialization and tested it.

@wesleycho
Copy link
Contributor

Right, so it turns out I broke that feature in this refactor - IMO we should try to fix this and retain that information in the date object constructed.

@Foxandxss
Copy link
Contributor Author

I am confused. This is what we achieve in here right? If you pass a baseDate to it, it will come just with the date part modified, the timezone won't change.

@wesleycho
Copy link
Contributor

Oh you're right, I should have took a second look at the code

@Foxandxss Foxandxss closed this in 21b2297 Oct 28, 2015
@Foxandxss Foxandxss deleted the fix/dateparserbasedate branch October 28, 2015 20:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants