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

Datepicker bugfixing #3288

Closed
wants to merge 2 commits into from
Closed

Datepicker bugfixing #3288

wants to merge 2 commits into from

Conversation

jmayday
Copy link

@jmayday jmayday commented Feb 11, 2015

I removed code allowing values which do not match format being parsed as dates.

@antoinepairet
Copy link

@jmayday Could you squash the commits to make only one. Thanks in advance

@jmayday
Copy link
Author

jmayday commented Feb 13, 2015

Squashed commits are available under branch https://github.com/jmayday/bootstrap/tree/datepicker-bugfixing-squashed and PR is here #3294

@jmayday
Copy link
Author

jmayday commented Feb 16, 2015

I've had a look at antoinepairet@4d97d7c and it doesn't work so well with change I proposed (having call to new Date(viewValue) removed). Of course I don't mean that content of commit antoinepairet@4d97d7c is broken. It's good work actually. I would just like to clarify some working combination of these two fixes.

So, let's describe it this way:

  • when my change is kept then date can't be initialized
  • when my change is ignored (we keep calling new Date(viewValue)) then we still get values like "1" parsed into the model as date which is not necessarily expected

You can use this plunker to compare the behaviour - http://plnkr.co/edit/HKALJPWWnpyU8nU8iAvX

@wesleycho
Copy link
Contributor

As mentioned in #2513, this fails some tests when based off of current master, and needs to be updated.

@wesleycho wesleycho added this to the Purgatory milestone Mar 30, 2015
@wesleycho wesleycho removed their assignment Mar 30, 2015
@chrisirhc
Copy link
Contributor

Closing as #3294 is a newer version of this PR.

@chrisirhc chrisirhc closed this Apr 6, 2015
@karianna karianna modified the milestones: 0.13.0, Purgatory Apr 7, 2015
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.

5 participants