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

Fixed booleanAttrs to sync model. #1057

Closed
wants to merge 1 commit into from
Closed

Fixed booleanAttrs to sync model. #1057

wants to merge 1 commit into from

Conversation

floydwch
Copy link

Current booleanAttrs can't sync model when asserted.

Using link instead of compile to fix this issue.

@floydwch
Copy link
Author

Isn't this considered a bug?

<div ng-app="" ng-init="slave=false">
  Check me to check both: <input id="checkMaster" type="checkbox" ng-model="master"><br/>
  <input id="checkSlave" type="checkbox" ng-checked="master" ng-model="slave">
  {{slave}}
</div>

Check on #checkMaster should both change ng-model="master" and ng-model="slave".

But #checkSlave set ng-model="slave" only change its ng-model when manually check on #checkSlave.

@mhevery
Copy link
Contributor

mhevery commented Aug 31, 2012

Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement).

CLA is important for us to be able to avoid legal troubles down the road.

For individuals (a simple click-through form):
http://code.google.com/legal/individual-cla-v1.0.html

For corporations (print, sign and scan+email, fax or mail):
http://code.google.com/legal/corporate-cla-v1.0.html

@floydwch
Copy link
Author

floydwch commented Sep 3, 2012

@mhevery I have signed it.

@mhevery
Copy link
Contributor

mhevery commented Sep 4, 2012

Hi @floydsoft, sorry, I took a better look at this pull request, and we think what you are trying to do does not make sense. ng-checked creates a one-way data-binding but ng-model creates two-way databinding. In essence you have two models connected to the same input element, and i think that that is just wrong. I think ng-model and ng-checked should be used exclusively, ie one or the other but not together.

I am going to close this PR without merging it. Let me know if yo feel otherwise.

@mhevery mhevery closed this Sep 4, 2012
@floydwch
Copy link
Author

floydwch commented Sep 5, 2012

@mhevery

So what's the right way to implement such requirement?

Why ng-checked just a one-way data-binding?

@mhevery
Copy link
Contributor

mhevery commented Sep 7, 2012

I think it should all be done through ng-model

On Tue, Sep 4, 2012 at 6:44 PM, floydsoft notifications@github.com wrote:

@mhevery https://github.com/mhevery

So what's the right way to implement such requirement?

Why ng-checked just a one-way data-binding?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1057#issuecomment-8285198.

@floydwch
Copy link
Author

floydwch commented Sep 8, 2012

@mhevery consider following example, slave1 and slave2 cannot be group checked by master, we should use ng-checked, but ng-checked conflicts with ng-model, which store slave1 and slave2's states.

How to solve it?

<div ng-app="" ng-init="master=false;slave1=false;slave2=false">
  Check me to check both: <input id="checkMaster" type="checkbox" ng-model="master"><br/>
  <input id="checkSlave" type="checkbox" ng-model="slave1">
  {{slave1}}
  <input id="checkSlave2" type="checkbox" ng-model="slave2">
  {{slave2}}
</div>

@floydwch
Copy link
Author

@mhevery

I've clarified my thought, and post on forum, the original version I tweaked had problem that it must include ngModel, but that's just wrong.

How about

      require: '?ngModel',
      link: function(scope, element, attr, ngModel) {
        scope.$watch(attr[normalized], function ngBooleanAttrWatchAction(value) {
          attr.$set(attrName, !!value);
          if(ngModel) {
            ngModel.$setViewValue(!!value);
          }
        });
      }

Making ngChecked work with ngModel, and I think it doesn't mess up something.

@derokorian
Copy link

I think the problem here is that ngChecked is one way - it reads a value and affects the checked status of the input its on. ngModel is two way - it reads a value and affects the input but it also reads the input and affects the value.

Therefore, I feel (and clearly others do as well), that the problem is that ngChecked changes the checked status of a checkbox. The model on that checkbox is supposedly directly tied to the checked status of it, however we now see the checked status changing, without the model changing. I don't see how allowing this functionality would cause multiple two way bindings on an element, as the checkbox cannot affect the value(s) of the expression passed to it in ngChecked.

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

Successfully merging this pull request may close these issues.

3 participants