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

Allow to use coma in numbers #25

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

GCorbel
Copy link

@GCorbel GCorbel commented Nov 19, 2016

In french, and in some other languages, coma are used instead of dos to separate numbers.

Not sure about the implementation but I didn't found an other way.

@offirgolan
Copy link
Collaborator

@GCorbel I agree with you, this is certainly an issue I havent faced yet. Im not really sure if this is the best approach though or if it will break anything upstream.

@GCorbel
Copy link
Author

GCorbel commented Nov 20, 2016

It works for me. We may have an option like allowComas: true. What you think ?

@GCorbel
Copy link
Author

GCorbel commented Nov 22, 2016

@offirgolan I need your feedback for one project. Please, let me know if you think my proposition is good or not.

@offirgolan
Copy link
Collaborator

@GCorbel I'm sorry but I'm on vacation and I dont have time to check this logic out for the next 2 weeks. If you are using this via ember-cp-validations, you have multiple ways of doing this with public API.

  1. You can create your own number validator by extending the existing one.
  2. You can use the value option to format the number string.

Hopefully that will unblock you until I have more time to verify this PR.

@acorncom
Copy link

@offirgolan This PR looks relatively straightforward, any chance we can get it merged?

@offirgolan
Copy link
Collaborator

@GCorbel can we put this behind an allowComas flag?

@GCorbel
Copy link
Author

GCorbel commented Jul 15, 2017

@offirgolan sorry for late answer. I will check it this weekend I hope.

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.

3 participants