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

feat(dateParser): add dateParser service #1874

Closed
wants to merge 1 commit into from

Conversation

bekos
Copy link
Contributor

@bekos bekos commented Mar 2, 2014

Will appreciate if anyone is interested to review or add tests.

@Foxandxss
Copy link
Contributor

Angular team is suggesting to not use the $ in non-core services (See here). We are using it on a lot of places and some of them ($modal) could bring breaking changes. I think that at least we should avoid creating new services with the $.

@bekos
Copy link
Contributor Author

bekos commented Mar 2, 2014

@Foxandxss This makes sense, thx.

@chrisirhc
Copy link
Contributor

Very nice. I just reviewed it. Clean implementation. Looks good to me.

@bekos
Copy link
Contributor Author

bekos commented Mar 4, 2014

I remove the $ from service as @Foxandxss suggested.

@chrisirhc Thx for the review and your time. I also added edge case validation, like 29/02/2014, 31 April etc, that cannot be "catched" with regex as it is now.

@bekos bekos changed the title (WIP) feat(dateParser): add dateParser service feat(dateParser): add dateParser service Apr 18, 2014
@bekos bekos closed this in bd2ae0e Apr 19, 2014
@pkozlowski-opensource
Copy link
Member

Yay! This should considerably cut down number of issues opened for date picker! Great job!

@cyberwombat
Copy link

Shouldn't this provide support to initialize the datepicker with a string value? It seems that the validator is getting in the way of that. If I add a directive such as:

angular.module('app').directive('datepickerPopup', [function() {
  return {
    restrict: 'EA',
    require: 'ngModel',
    link: function(scope, element, attrs, ngModel) {
      ngModel.$validators = [];
    }
  };
}]);

I can now load using a date string and it will still validate input. Not sure what is doing the validation but it now seems to a) accept an ISO string as input and b) prevent me from entering anything but a valid date in the correct format. I admit I am a little confused as to where this validator is coming from but that is the apparent effect I see.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants