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

feat(ngModelOptions): add allowInvalid option - #8313

Closed
shahata wants to merge 1 commit into
angular:masterfrom
shahata:refactor-validation
Closed

feat(ngModelOptions): add allowInvalid option#8313
shahata wants to merge 1 commit into
angular:masterfrom
shahata:refactor-validation

Conversation

@shahata

@shahata shahata commented Jul 24, 2014

Copy link
Copy Markdown
Contributor

This option allows to write invalid values to the model instead of having them become undefined.

Closes #8290

@mary-poppins

mary-poppins commented Jul 24, 2014

Copy link
Copy Markdown

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#8313)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@shahata

shahata commented Jul 24, 2014

Copy link
Copy Markdown
Contributor Author

The main idea here is that $parsers no longer change the value to undefined in case the value is invalid. Instead, we let $$runValidators do that in turn - and this is a refactoring we can do without breaking a single test. From there, adding the option to allow writing invalid values to the model is trivial.

This is still a work in progress since I haven't added any docs yet, but I wanted to put it here to get some feedback before I continue.

@shahata shahata added cla: yes and removed cla: no labels Jul 24, 2014
@Narretz

Narretz commented Jul 24, 2014

Copy link
Copy Markdown
Contributor

Hey @shahata, thanks for working on this. I think we should go one step further and completely decouple $parsers and $validators. With $validators, $parsers should now always return a representation of the model. The problem with date and number is obviously that we have to have some sort of validation to be able to parse them, but that also means the $validators can cleanly expect a Date object respectively a number.
I've also seen that @matsko has expanded his PR for async validation (https://github.com/angular/angular.js/pull/8267/files) and the validation API has changed again. We'll have to wait how this one plays out.

@btford btford added this to the Backlog milestone Jul 24, 2014
@jgrenat

jgrenat commented Aug 11, 2014

Copy link
Copy Markdown

Hey! Is there any news about this PR?

@shahata

shahata commented Aug 12, 2014

Copy link
Copy Markdown
Contributor Author

Waiting for #8267

@btford btford removed the gh: PR label Aug 20, 2014
@shahata

shahata commented Aug 30, 2014

Copy link
Copy Markdown
Contributor Author

I've rebased this since #8267 was merged

@matsko matsko self-assigned this Aug 30, 2014
@IgorMinar IgorMinar modified the milestones: 1.3.0-rc.1, Backlog Aug 30, 2014
@IgorMinar

IgorMinar commented Aug 30, 2014

Copy link
Copy Markdown
Contributor

I'm fine with this making it into rc.1

cc: @tbosch

tbosch
tbosch reviewed Sep 2, 2014
View reviewed changes
Comment thread test/ng/directive/inputSpec.js Outdated

@tbosch tbosch Sep 2, 2014

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.

I don't understand this test. If the parser decides that the value is not parseable, we still don't get a value in the model, right?

@shahata shahata Sep 2, 2014

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.

Right. This is a relic from when we still had validators inside $parsers pipeline. This test now does exactly the same as the first one. I removed it.

@tbosch

tbosch commented Sep 2, 2014

Copy link
Copy Markdown
Contributor

Could you add the following tests as well:

  • it('should not assign not parsable values to the scope if if allowInvalid is true'), test by using input[number] and assign a string that is not a number...

tbosch
tbosch reviewed Sep 2, 2014
View reviewed changes
Comment thread test/ng/directive/inputSpec.js Outdated

@tbosch tbosch Sep 2, 2014

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.

please change to: it('should assign invalid values to the scope if allowInvalid is true'...

@shahata shahata Sep 2, 2014

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.

Sure. Done.

@tbosch tbosch assigned tbosch and unassigned matsko Sep 3, 2014
@tbosch tbosch mentioned this pull request Sep 5, 2014
Closed
@tbosch

tbosch commented Sep 5, 2014

Copy link
Copy Markdown
Contributor

Note: I am in the process of refactoring ngModelController and formController to simplify control flow and eliminate bugs. See #8941.
This needs to wait until that other PR is in.

To be discussed in this PR:
Probably also remove ctrl.$$invalidModelValue and always write to $modelValue immediately (but don't write to the scope immediately)

@shahata

shahata commented Sep 5, 2014

Copy link
Copy Markdown
Contributor Author

np, i'll take care of it when rebasing

@shahata

shahata commented Sep 9, 2014

Copy link
Copy Markdown
Contributor Author

Rebased with #8941
I'll look into removing ctrl.$$invalidModelValue tonight

@Narretz

Narretz commented Sep 9, 2014

Copy link
Copy Markdown
Contributor

I have a PR for removing ctrl.$$invalidModelValue. I can put it up tonight. There is one thing that I'm not sure about, so I'd welcome your input

This was referenced Sep 9, 2014
Closed
Closed
@jeffbcross jeffbcross modified the milestones: 1.3.0-rc.1, 1.3.0-rc.2 Sep 9, 2014
@shahata
feat(ngModelOptions): add allowInvalid option
1cc1b85 
This option allows to write invalid values to the model instead of having them become undefined.

Closes #8290
@Narretz Narretz mentioned this pull request Sep 9, 2014
Closed
tbosch pushed a commit to tbosch/angular.js that referenced this pull request Sep 9, 2014
@shahata @tbosch
feat(ngModelOptions): add allowInvalid option
d7bfae2 
This option allows to write invalid values to the model instead of having them become undefined.
Use this together with calling `ctrl.$setValidity` directly for displaying errors
from serverside validation.

Closes angular#8290
Closes angular#8313
@tbosch tbosch closed this in 3c538c1 Sep 9, 2014
mgallag pushed a commit to mgallag/angular.js that referenced this pull request Sep 10, 2014
@shahata Michael Gallagher
feat(ngModelOptions): add allowInvalid option
525e7ac 
This option allows to write invalid values to the model instead of having them become undefined.
Use this together with calling `ctrl.$setValidity` directly for displaying errors
from serverside validation.

Closes angular#8290
Closes angular#8313
tbosch pushed a commit that referenced this pull request Sep 10, 2014
@shahata @tbosch
feat(ngModelOptions): add allowInvalid option
a3d7934 
This option allows to write invalid values to the model instead of having them become undefined.
Use this together with calling `ctrl.$setValidity` directly for displaying errors
from serverside validation.

Closes #8290
Closes #8313
Closes #9016
mgallag pushed a commit to mgallag/angular.js that referenced this pull request Sep 10, 2014
@shahata Michael Gallagher
feat(ngModelOptions): add allowInvalid option
e4e63a3 
This option allows to write invalid values to the model instead of having them become undefined.
Use this together with calling `ctrl.$setValidity` directly for displaying errors
from serverside validation.

Closes angular#8290
Closes angular#8313
mgallag pushed a commit to mgallag/angular.js that referenced this pull request Sep 10, 2014
@shahata Michael Gallagher
feat(ngModelOptions): add allowInvalid option
eeddb1d 
This option allows to write invalid values to the model instead of having them become undefined.
Use this together with calling `ctrl.$setValidity` directly for displaying errors
from serverside validation.

Closes angular#8290
Closes angular#8313
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@tbosch tbosch

Labels

cla: yes component: misc core frequency: low severity: confusing type: feature

Projects

None yet

Milestone

1.3.0-rc.2

Development

Successfully merging this pull request may close these issues.

Add option to write invalid $modelValues to scope

9 participants

@shahata @mary-poppins @Narretz @jgrenat @IgorMinar @tbosch @matsko @jeffbcross @btford