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

Introduce ignoreWhitespace in presence validator #173

Merged
merged 1 commit into from
Apr 25, 2016
Merged

Introduce ignoreWhitespace in presence validator #173

merged 1 commit into from
Apr 25, 2016

Conversation

krasnoukhov
Copy link
Contributor

@krasnoukhov krasnoukhov commented Apr 20, 2016

This allows using a validator for a case when value should not consist only of space(s).

@aaronbhansen
Copy link
Contributor

I feel like this should be a configuration option for the presence validator. Something like

validator('presence', { ignoreWhitespace: true })

or

validator('presence', { ignoreBlank: true })

Since this would change existing functionality for people already using it today.

@krasnoukhov krasnoukhov changed the title Use Ember.isPresent in presence validator Introduce ignoreBlank in presence validator Apr 20, 2016
@krasnoukhov
Copy link
Contributor Author

Makes sense, thanks. I've updated the commit. CI is failing though, but it does not look like my change broke it, so not sure what can I do

@krasnoukhov
Copy link
Contributor Author

CI is green now!

@offirgolan
Copy link
Collaborator

@krasnoukhov thank you for this feature! I think I prefer "ignoreWhitespace" more just because it's a little more intuitive, but I'm not sure if that's more misleading than ignoreBlank. May I ask why you chose ignoreBlank? Besides that, if you can just add the option details in the docs (see addon/validators/format), I believe I can just go ahead and merge this baby 😸

@krasnoukhov
Copy link
Contributor Author

@offirgolan No problem! Well ignoreBlank is just shorter, but I've updated a commit with a new name and polished doc.

@krasnoukhov krasnoukhov changed the title Introduce ignoreBlank in presence validator Introduce ignoreWhitespace in presence validator Apr 20, 2016
@offirgolan
Copy link
Collaborator

Oh okay I was just curious. After some thought, I believe ignoreWhitespace
sort of implies that it will ignore all whitspace characters in a string.
Maybe ignoreBlank was the better choice but regardless, I can always make
that change after I merge 😊. I'm currently on vacation so I'll merge this
along with some open PRs early next week and issue a release.

On Thursday, April 21, 2016, Dmitry Krasnoukhov [email protected]
wrote:

@offirgolan https://github.com/offirgolan No problem! Well ignoreBlank
is just shorter, but I've updated a commit with a new name and polished doc.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#173 (comment)

Thanks,
Offir Golan

@krasnoukhov
Copy link
Contributor Author

Sure, I don't mind at all, maybe you'll think of even better name for that! Thanks and have a good rest of the week.

@blimmer
Copy link
Contributor

blimmer commented Apr 21, 2016

LGTM

@offirgolan offirgolan merged commit bba76b8 into adopted-ember-addons:master Apr 25, 2016
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.

4 participants