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

input[type=radio] with object behind ng-model #6668

Closed
wants to merge 1 commit into from

Conversation

serzh
Copy link

@serzh serzh commented Mar 13, 2014

Request Type: bug

How to reproduce: Use object for ng-model for input[radio]

Component(s): misc core

Impact: medium

Complexity: small

This issue is related to:

Detailed Description:

Other Comments:

If you use object as models with radio, then the $render method won't be ever called, because of {} == {} is always false. So we should use angular.equals instead of ==.

@mary-poppins
Copy link

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 (#6668)

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!

@caitp
Copy link
Contributor

caitp commented Mar 13, 2014

This change has come up a few times before, I recall there are a few concerns, one of which is the potential performance cost (which may be fine for you, but may be harder on an different applications).

So there are actually things you can do to make this nicer, like use the value as an property name in an object containing the object values.

I'm not really opposed to the change, but these things have been raised before, so it might be better to find a solution which provides a sort of compromise

@serzh
Copy link
Author

serzh commented Mar 14, 2014

What if we use angular.isObject before use == or angular.equals in $render?

@runk
Copy link
Contributor

runk commented Mar 17, 2014

It would be nice to compare objects correctly 👍 Having the same issue with ngRadio component.

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (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 us 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.

@serzh serzh added cla: no and removed cla: yes labels Mar 18, 2014
@serzh
Copy link
Author

serzh commented Mar 18, 2014

I have signed CLA.

@mary-poppins
Copy link

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).

@Narretz
Copy link
Contributor

Narretz commented Sep 13, 2015

This will be fixed on the ngModel level, see #12783

@Narretz Narretz closed this Sep 13, 2015
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.

6 participants