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

feat(datepicker): add suppression of validation for #3836 #4115

Closed
wants to merge 1 commit into from
Closed

feat(datepicker): add suppression of validation for #3836 #4115

wants to merge 1 commit into from

Conversation

Blackbaud-PatrickOFriel
Copy link
Contributor

This adds the $datepickerSuppressError value provider which has an isSuppressed property to allow the option to suppress the $log error in DatepickerController. This was made to address #3836

@@ -1,5 +1,9 @@
angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootstrap.position'])

.value('$datepickerSuppressError', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just make it a boolean - no need for an object here.

@wesleycho
Copy link
Contributor

Just make all changes related to my comment and this PR should be good to get in.

@wesleycho wesleycho added this to the 0.13.3 (Fixes) milestone Aug 6, 2015
@Blackbaud-PatrickOFriel
Copy link
Contributor Author

The only reason I made it an object is because when it's a literal, changes you make upon injection don't take effect (setting $datepickerSuppressError to true). You could only change the value in the config phase using $provide. If that's the route I should take, then I'll have to make the test a bit different since you can't use $provide after you've used inject.

@wesleycho
Copy link
Contributor

I think that is fine - there really isn't a great use case for toggling suppression on or off once the app is running.

@Blackbaud-PatrickOFriel
Copy link
Contributor Author

Okay I'll go ahead and make the change then

@Blackbaud-PatrickOFriel
Copy link
Contributor Author

So in order to test properly I had to wrap the other tests in their own describe block so that I could override the $provide for my test cases before the inject function was ever called. And now the diff tool makes it look like I deleted everything and added it back...

@wesleycho
Copy link
Contributor

That's perfectly fine

@wesleycho
Copy link
Contributor

Can you squash your commits into one?

@Blackbaud-PatrickOFriel
Copy link
Contributor Author

Squash complete

@wesleycho
Copy link
Contributor

LGTM - merging shortly. Thanks for the work!

@wesleycho wesleycho closed this in bab1d37 Aug 7, 2015
@Blackbaud-PatrickOFriel
Copy link
Contributor Author

No problem! Thanks for this entire library, it's great!

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.

3 participants