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

Commit

Permalink
fix(autocomplete): adds watcher for ngDisabled rather than 2-way binding
Browse files Browse the repository at this point in the history 
Closes #2160
  • Loading branch information
Robert Messerle committed Apr 24, 2015
1 parent ef0dce0 commit 973a2fc
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/components/autocomplete/js/autocompleteDirective.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ function MdAutocomplete ($mdTheming, $mdUtil) {
noCache: '=?mdNoCache',
itemChange: '&?mdSelectedItemChange',
textChange: '&?mdSearchTextChange',
isDisabled: '=?ngDisabled',
minLength: '=?mdMinLength',
delay: '=?mdDelay',
autofocus: '=?mdAutofocus',
Expand Down Expand Up @@ -140,6 +139,9 @@ function MdAutocomplete ($mdTheming, $mdUtil) {
};

function link (scope, element, attr) {
if (attr.ngDisabled) {
scope.$parent.$watch(attr.ngDisabled, function (val) { scope.isDisabled = val; });
}
scope.contents = attr.$mdAutocompleteTemplate;
delete attr.$mdAutocompleteTemplate;

Expand Down

6 comments on commit 973a2fc

@gkalpak
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this change ?

And why $parent ? And why not attrs.$observe ?

I am a curious person, I know :)

@robertmesserle
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it's a bug in Angular or due to the complexity of this component, but using $observe caused it to be evaluated in the wrong scope.

@gkalpak
Copy link
Member

Choose a reason for hiding this comment

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

Yes, obviously :) (it has an isolate scope).
But what was wrong with the previous implementation (2-way data-binding) as long as you don't try to "write back".

@robertmesserle
Copy link
Contributor

@robertmesserle robertmesserle commented on 973a2fc Apr 30, 2015 via email

Choose a reason for hiding this comment

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

@tkrotoff
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not watch ngDisabled directly. ng-disabled sets disabled for you (either disabled="disabled" => true ; or nothing => false). It is intended for the users not for the directive implementation. The user can use ng-disabled (<fooBar ng-disabled="ctrl.myBooleanVariable">) or directly disabled (<fooBar disabled>).

A better way (taken from https://github.com/tkrotoff/ui-select/blob/master/src/select.js#L220) is to simply watch disabled attribute:

attrs.$observe('disabled', function() {
  scope.disabled = attrs.disabled ? true : false;
});

See also my remarks on #2619: ng-checked, ng-disabled, ng-src... they all work the same way.

@ThomasBurleson
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkrotoff - we do watch disabled in the Select component.
@robertmesserle - should we change autocomplete to match that approach?

Please sign in to comment.