Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngModelController.$validators pipeline and refactoring - #7377

Closed
matsko wants to merge 6 commits into
angular:masterfrom
matsko:validators
Closed

matsko wants to merge 6 commits into
angular:masterfrom
matsko:validators

Conversation

@matsko

@matsko matsko commented May 7, 2014

Copy link
Copy Markdown
Contributor

Fixes #6750
Fixes #6304
Fixes #5164

@matsko matsko added this to the 1.3.0 milestone May 7, 2014
@matsko matsko changed the title ngModelController.$validators pipeline ngModelController.$validators pipeline and refactoring May 7, 2014
Comment thread src/ng/directive/input.js Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this is wrong; it seems like this says that if you have two validators, and the first one returns false, but the second one returns true, that the control will be set to valid. Am I missing something here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only if they're all valid will the control be valid, afaik

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each validator has a different name, so there is no problem here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's an AND operation when $valid is determined.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I looked over the code for $setValidity and discovered it was keeping track of how many times the control was valid vs. invalid. My mistake.

@Narretz

Narretz commented May 7, 2014

Copy link
Copy Markdown
Contributor

What is the relationship between $validators and setting the model & view values? Are they still only set when it is valid?

@matsko

matsko commented May 7, 2014

Copy link
Copy Markdown
Contributor Author

@Narretz validations will happen whenever the model value changes. It will always be the parsed value. The view value is ignored.

@Narretz

Narretz commented May 14, 2014

Copy link
Copy Markdown
Contributor

A minor thing: since validations are not short-circuiting anymore, pipeline isn't a very good description. It may confuse people who are familiar with parsers / validators pipeline.

@matsko

matsko commented May 14, 2014

Copy link
Copy Markdown
Contributor Author

@Narretz yes that is true. Maybe queue would be better.

@IgorMinar

Copy link
Copy Markdown
Contributor

to summarize, I have these questions:

  • async validation
    • why do we need it?
    • why is it implemented via postDigest?
  • pristine
    • why did the behavior change?
  • minlenght/maxlength
    • shouldn't the max/min value be dynamically updated?
  • validated value
    • previously it was possible to validate the value either before parsing (viewValue) or after parsing (modelValue). now it's only modelValue and the validator has no access to the viewValue. Is this not a problem?
  • modelValue update
    • previously the modelValue would not be updated if the value is invalid, this is not the case any more, which means that apps now need to guard themselves against invalid values. This is a major breaking change. Why?

@Narretz

Narretz commented May 21, 2014

Copy link
Copy Markdown
Contributor

Regarding model updates when invalid: The intention is that sometimes you want the value in the model even though it is invalid (same goes for model -> view and formatters). But this could be made configurable.

Updating the model also means that you don't see validation errors before the model is updated, which means consistency with the new model update options. A good example is entering an email address, and getting the 'invalid' error message while you are typing, which wouldn't happen with a debounce.

However that this is unflexible, as sometimes you want immediate feedback.. @shahata has a commit here that defines validateOn options similar to the model updates: #7414, so validation errors can be displayed before model updates are made. Maybe this can be integrated - which would mean $validators use the (parsed) viewValue again

@btford btford modified the milestones: 1.3.0-beta.11, 1.3.0-beta.10 May 23, 2014
matsko added 6 commits June 9, 2014 22:16
BREAKING CHANGE:

If an expression is used on ng-pattern (such as `ng-pattern="exp"`) or on the
pattern attribute (something like on `pattern="{{ exp }}"`) and the expression
itself evaluates to a string then the validator will not parse the string as a
literal regular expression object (a value like `/abc/i`).  Instead, the entire
string will be created as the regular expression to test against. This means
that any expression flags will not be placed on the RegExp object. To get around
this limitation, use a regular expression object as the value for the expression.

    //before
    $scope.exp = '/abc/i';

    //after
    $scope.exp = /abc/i;
@matsko

matsko commented Jun 13, 2014

Copy link
Copy Markdown
Contributor Author

MERGED

@matsko

matsko commented Jun 13, 2014

Copy link
Copy Markdown
Contributor Author

Landed as 1be9bb9

@matsko matsko closed this Jun 13, 2014
@matsko
matsko deleted the validators branch June 13, 2014 01:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

8 participants