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

avoid setting ngModelController to $dirty on initialization. #30

Closed
wants to merge 1 commit into from

Conversation

marbletravis
Copy link
Contributor

I know this can't be merged, and needs some cleanup, (not sure what happened to the min file). But would like to discuss if this can (with cleanup) be merged.

See issues #29

Travis

@PowerKiKi
Copy link
Contributor

Could you elaborate on why you are not sure if it should be merged ? It seems to be a good idea. Did you forsee compatibility issues or edge cases troubles ?

If you clean up, and add tests I would merge.

@marbletravis
Copy link
Contributor Author

@PowerKiKi My only concern with merging, was getting the ui-mask.js.min.js file name issue resolved in the dist folder. I should have time to resolve that today and update the pull request.

@PowerKiKi
Copy link
Contributor

Ok then. If for some reason you can't figure out the dist thing, you can always submit a PR with only the src changed, and I'll take care to build dist when releasing.

Btw, I just updated dev dependencies to latest version, that may help you. I'd suggest you rebase your PR on master and try again...

@marbletravis
Copy link
Contributor Author

@PowerKiKi it looks like the code around initPlaceholder has changed since I made my edits, doesn't look like it would be an issue now that we are not calling eventHandler. I can confirm tomorrow that its not an issue, but I would be surprised if it is. Looks like this pull request might be overcome by events.

if (maskProcessed) {
  iElement.val(maskValue(unmaskValue(iElement.val())));
}

versus

if (maskProcessed) {
   eventHandler({}, true);
}

@marbletravis
Copy link
Contributor Author

Closing this pull request, this does not seem to be an issue with the latest code changes.

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

Successfully merging this pull request may close these issues.

2 participants