Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Update modules/directives/mask/mask.js #357

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

This new version accepts a parameter to remove or not the mask as it is updated to the model and also make it more compatible with uiDate Controller

Update modules/directives/mask/mask.js
This new version accepts a parameter to remove or not the mask as it is updated to the model and also make it more compatible with uiDate Controller

@getuliojr thnx for your PR. Would you mind adding tests to expose what you are trying to fix with your change?

@ProLoser ProLoser commented on the diff Jan 16, 2013

modules/directives/mask/mask.js
@@ -25,13 +39,20 @@ angular.module('ui.directives').directive('uiMask', [
return isValid ? value : undefined;
});
- /* When keyup, update the view value
- */
- element.bind('keyup', function () {
- $scope.$apply(function () {
- controller.$setViewValue(element.mask());
- });
- });
+ /* When changed, update the view value but only if it is not been used with the directive uiDate
+ uiDate will parse as a date so it is better format for dates, instead of string.
+ */
+ if ((attrs["uiDate"] == undefined) && (attrs["uiDateFormat"] == undefined)) {
@ProLoser

ProLoser Jan 16, 2013

Owner

There's no reason / need to check for uiDateFormat since this affects only the model value and won't / shouldn't be used without uiDate, right?

UiDateFormat can be set out, the objective to make it more compatible with uiDate that will parse as DATE what is more expected to be used instead of STRING that this model would be using. I am setting a plnkr now to show it.

Here is a working example of the changes, hope it can explain by itself

http://plnkr.co/edit/Dq14NVBCWdL5ui1cRzh9?p=preview

Owner

ProLoser commented Jan 16, 2013

  1. You should probably be hooking into the input event instead of change
  2. Why do you even need to hook into this event? Doesn't $render do this automatically?

It was wired on keyup on original code to parse the values back to the model. I did not see a reason to be listening to keyup so instead I changed to bind to 'change' event to parse the values back to the model.

Owner

ProLoser commented Jan 16, 2013

Does the mask plugin preventDefault() or something? Why is it necessary to explicitly set the model?

Dean Sofer
DeanSofer.com
714.900.2254

On Wednesday, January 16, 2013 at 2:17 PM, getuliojr wrote:

It was wired on keyup on original code to parse the values back to the model. I did not see a reason to be listening to keyup so instead I changed to bind to 'change' event to parse the values back to the model.


Reply to this email directly or view it on GitHub (angular-ui#357 (comment)).

First I did not want to make a huge rewrite in the diretive in order to not brake it, since it is the first time I am contributting and as so I need to take small steps at a time.

Second, since it you might set the model value with or without a mask you can´t have it get the defaultvalue set. You do need to check this step.

But I do think preventDefault might be been used, since it is a plugin. Did not check it though.

Member

shaungrady commented Mar 16, 2013

uiMask has since been refactored, this issue is no long applicable. It will be in the next AngularUI release.

@shaungrady shaungrady closed this Mar 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment