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

fix(select): ngRequired invalidate when modelValue is empty array #5337

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Dec 9, 2013

When multiple attribute is set, and the model value is an empty array, invalidate the form control.

TODO: ngModelWatch should probably do a bit more work to see if the model value is equal or not. (see comment in selectSpec.js)

Fixes #2365

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

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!

@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.


scope.$apply(function() {
// ngModelWatch does not set objectEquality flag
// array must be replaced in order to trigger $formatters
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this little jig be turned into a separate bug? Perhaps ngModelController could compare value.length to the old length (and try not to break in the case of scalars, of course) --- to avert this somewhat. Or perhaps just use the objectEquality flag?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should open a new bug for that. See #5449

@caitp
Copy link
Contributor Author

caitp commented Dec 9, 2013

@IgorMinar I know that these are automated messages, however I did just sign again today just in case the old signature really did vanish.

It is signed as Caitlin Potter

When `multiple` attribute is set, and the model value is an empty array,
invalidate the form control.
@IgorMinar
Copy link
Contributor

@caitp the email addresses don't match. look at the email at https://github.com/angular/angular.js/pull/5337.patch

it's different from the one in the spreadsheet.

and yes, the comments are automated so it will keep on bugging you until you sign the CLA with the right email or change all pull requests to use the other email (the former is less hassle).

sorry about the spam, but given how many PRs we deal with, we are automating as much as we can and sometimes it results in "spammy" behavior. we are trying hard to avoid it as much as possible.

@IgorMinar
Copy link
Contributor

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@caitp
Copy link
Contributor Author

caitp commented Dec 10, 2013

Yeah as I said in matsko's patch, I understand why it's being done, so of course there is no need for apologies :) I didn't realize the email address was being used rather than the author name, however.

Anyways, re-signed with the legacy email address, cheers.

@ghost ghost assigned petebacondarwin Dec 12, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
When `multiple` attribute is set on a `<select>` control and the model value is an empty array,
we should invalidate the control.  Previously, this directive was using incorrect logic for
determining if the model was empty.

Closes angular#5337
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
When `multiple` attribute is set on a `<select>` control and the model value is an empty array,
we should invalidate the control.  Previously, this directive was using incorrect logic for
determining if the model was empty.

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

Successfully merging this pull request may close these issues.

ngRequired: suggest mark empty arrays as invalid
4 participants