Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Pristine is not preserved #41

Closed
msimpson opened this issue Sep 4, 2015 · 23 comments · Fixed by #69
Closed

Pristine is not preserved #41

msimpson opened this issue Sep 4, 2015 · 23 comments · Fixed by #69
Labels

Comments

@msimpson
Copy link

msimpson commented Sep 4, 2015

Regardless of interaction, ui-mask immediately destroys the pristine/untouched state of input elements. In contrast, default angular directives--such as ng-required--preserve the pristine/untouched state of input elements, even when focus or blur events occur, as long as the value of the input has never changed from its original state.

For example, if I were to construct the following:

<input 
    id="ssn" 
    name="ssn" 
    type="text" 
    placeholder="123-45-6789"
    ng-model="ssn"
    ng-required="true">

The initial state of the input element, as shown by its class names, would be:

ng-pristine ng-untouched ng-invalid ng-invalid-required

And if I were to focus this input and subsequently blur it, without typing anything into it (i.e. not changing its value), both the pristine and untouched states would be preserved and I could check $dirty to hide validation errors until the input has been truly changed.

However, if I were to construct the following:

<input 
    id="ssn" 
    name="ssn" 
    type="text" 
    placeholder="123-45-6789"
    ui-mask-placeholder
    ui-mask="999-99-9999" 
    ng-model="ssn"
    ng-required="true">

Upon focus the ui.mask directive would immediately update both the input's value and the viewValue to display the mask. This will then cause the field to lose both the pristine and untouched state. And if the surrounding form was employing the ng-model option for debounce, there would be no $dirty check to hide ng-required errors.


A possible solution to this issue would be to alter both the eventHandler and blurHandler of the ui.mask source to take the following condition into consideration:

if (val.length == 0 && controller.$pristine) return;

This would essentially stop the mask from displaying on focus, but also preserve these states if a user was simply tabbing through the form, for instance.

@marbletravis
Copy link
Contributor

@msimpson I had a similar issue, with a different potential solution (#30) however the issue seemed to have been resolved in version 1.4.2. What version are you working on?

@marbletravis
Copy link
Contributor

@msimpson it also may have something to do with this
d120b8c

@PowerKiKi
Copy link
Contributor

Closing since @marbletravis gave solutions

@msimpson
Copy link
Author

I'm on version 1.4.5, have been from the start, and neither comments are solutions. I don't have the time to make a demo of this issue. But if someone does, try it out--it's there.

@marbletravis
Copy link
Contributor

@msimpson I wonder, is the element gaining focus? One solution I have had to use, not sure if its something that needs to change, is to limit the events to handle to ['input', 'keyup'].

The other day I was wondering (have not had time to look into it) if we can make the mask process (handle events) from focus and click without dirtying the model. For us only handling input and keyup is sufficient.

To override the config you just need to re-declare the config, we do it in our app.js

angular.module('...', []).value('uiMaskConfig', {
    maskDefinitions: {
        '9': /\d/,
        'A': /[a-zA-Z]/,
        '*': /[a-zA-Z0-9]/
    },
    clearOnBlur: false,
    eventsToHandle: ['input', 'keyup']
})

@msimpson
Copy link
Author

@marbletravis
Dropping focus does work around the issue, yes. But, again, this needs to be fixed at its core.

@marbletravis
Copy link
Contributor

@msimpson Sorry, my bad for not re-reading the whole op.

I am not sure what purpose focus/click serve, for us we didn't see a change in any behavior when switching to just input/keyup. I may have some spare cycles friday to look at the core issue, but wondering if you or @PowerKiKi can offer any context as to why we handle those events anyway.

@msimpson
Copy link
Author

@marbletravis
I'm not sure why those events are watched, either. Perhaps just paranoia over the slew of events signaling modification. Also, I found that while removing click and focus does resolve the pristine/untouched issue for mouse interaction it fails when tabbing through the fields as keyup is caught instead. Therefore, I still have no solid workaround.

@PowerKiKi
Copy link
Contributor

@marbletravis sorry, I don't have any explanation either. Your best bet is to dig through git blame.

@petejordan
Copy link

suffering with this ui-mask issue dirtying an otherwise prisitine page when nothing happens but a collapse / expand for e.g.
still very much open issue (in Object {full: "1.4.5", major: 1, minor: 4, dot: 5, codeName: "permanent-internship"} at least.
what's the fix / work-around / hack / ???

@marbletravis
Copy link
Contributor

@petejordan @msimpson I haven't had much time to devote to this. But I did implement some changes on my fork, which if it works I will create a pull request into the main repo. I can test this tomorrow afternoon, (I don't have a good test setup at home) but in the meantime if either of you want to test it out you can pull from here.

https://github.com/marbletravis/ui-mask

If you are using bower, see this SO https://stackoverflow.com/questions/19348076/installing-a-dependency-with-bower-from-url-and-specify-version

If you have time to verify please let me know the outcome.

@marbletravis
Copy link
Contributor

No go, it still has some $pristine issues. I will keep looking at it. Please be patient, I can't work on this at work much (not part of my main tasks right now) and have a family that occupies the rest of my time. I think I can look at this some more Friday.

@marbletravis
Copy link
Contributor

@PowerKiKi should we reopen this?

@petejordan
Copy link

Yes! Yes! Please re-open.
The mask dirty seems OK but form still dirty.
Thx much,
petejordan
On Oct 7, 2015 3:54 PM, "marbletravis" notifications@github.com wrote:

No go, it still has some $pristine issues. I will keep looking at it.
Please be patient, I can't work on this at work much (not part of my main
tasks right now) and have a family that occupies the rest of my time. I
think I can look at this some more Friday.


Reply to this email directly or view it on GitHub
#41 (comment).

@PowerKiKi
Copy link
Contributor

I reopen as requested by @marbletravis, but I won't be working on it.

Thanks @marbletravis for your past and future work on this issue.

@PowerKiKi PowerKiKi reopened this Oct 12, 2015
@PowerKiKi PowerKiKi added the bug label Oct 12, 2015
@bradrich
Copy link

I am also having this issue, but it is localized to IE only. Is this the case with everyone else's issues?

@petejordan
Copy link

Chrome also has problem of false dirty.
On Oct 13, 2015 12:32 PM, "Brad Richardson" notifications@github.com
wrote:

I am also having this issue, but it is localized to IE only. Is this the
case with everyone else's issues?


Reply to this email directly or view it on GitHub
#41 (comment).

@marbletravis
Copy link
Contributor

@bradrich I recall having some issues with IE, and I recall seeing a pull request to fix them. But can't find it anymore. Are you using latest?

@bradrich
Copy link

1.4.7, yes, it is the latest version. I am not seeing the error in Chrome at this current moment, only IE.

@blake-nouribekian
Copy link

Is anyone close to fixing this issue? All of the states are messed up. It should set them just like any other input would that is not using ui-mask on it. Right when I click or tab to the field I get my required error message even though it should only display the message when the field is dirty or has an error but since ui-mask is changing dirty right when the field is focused on it makes this impossible.

@marbletravis
Copy link
Contributor

@blake-nouribekian There is a pull request in with a fix. Just waiting for approval. You can switch to my fork temporarily if you need it faster.

#51

@blake-nouribekian
Copy link

Great... Thanks so much! I'm launching a business/website very soon so the less issues the better

@Tim91084
Copy link
Contributor

I've got another pull request in for this issue, #57

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

Successfully merging a pull request may close this issue.

7 participants