networkteam / ember-cli-maskedinput

Masked <input/> Ember component based on inputmask-core (no jQuery)
MIT License
25 stars 11 forks source link

Component conflicting with Validations #12

Closed bluscreen closed 7 years ago

bluscreen commented 7 years ago

Hello,

first of all thank you for putting this up. I have two feature requests to make amounts flexible, just like in regex. Also I would say it is rather bad practice to actually put underscores into the property.

Let me explain my use case:

I'm formatting Swiss license plate number which has to meet following regex: /^[A-Z]{2}\s\d{1,6}$/,

thus, I'm using ember-cp-validations to validate it with a regex.

Now adding the input mask plugin with mask AA 111111 results in an invalid property since your plugin actually writes underscores into the model for open fields.

Ergo: unless I fill up all 6 numbers, i get these underscores in my ember data property which the validator will not like at all.

First I thought I might just need to change the regex to this regex: /^[A-Z]{2}\s[\d_]{1,6}$/,

then however I found out that the underscores are actually not removed from the property and are even going to be persisted like that...

For now i will have to put up my own solution, but it's an idea that would make your plugin much more robust and usable.

Cheers Daniel

hlubek commented 7 years ago

Hi, I see the issue. The underlying library https://github.com/insin/inputmask-core does not support a variable number of characters in the pattern. There is however an option isRevealingMask which should work if the variable amount is at the end, so no underscores are returned in the value. This option is not yet implemented in the addon, but that's a simple change.

Also I would say it is rather bad practice to actually put underscores into the property.

Well, inputmask-core returns the value filled with placeholders, so there should always be a separate validation before the value is applied to a property (e.g. using on-change). But this should be documented better.

hlubek commented 7 years ago

See #13 to track the issue for the new option.