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

feat(select): add $optionChangeListeners to selectDirective controller #9285

Closed
wants to merge 3 commits into from
Closed

feat(select): add $optionChangeListeners to selectDirective controller #9285

wants to merge 3 commits into from

Conversation

pocesar
Copy link
Contributor

@pocesar pocesar commented Sep 26, 2014

Adds $optionChangeListeners to the selectDirective controller, this is fully backward compatible and has no impact on performance whatsoever if the dev decides not to use it, and has no impact on existing code.

The reasoning behind this feature is that ng-options expression is un$parseable from outside the optionDirective, and you can't have the current set resulting from the ng-options expressions without relying on a scope. This forces the dev to create a custom directive to watch for changes on a filtered variable from the underlaying scope (like ng-options="item.text for item in (filtered = (items | filter:fn1 | filter:fn2))" and it's not portable between directives, like $viewChangeListeners in NgModelController.
This makes isolated select controllers easier to accomplish and scope agnostic, aka web components style.

This also encourages future proof directive code, since it won't depend on the DOM or manually crafted enforced scopes, nor needs to watch over collections (that increases $digest overhead).

Pseudo-code:

.directive('selectPlugin', function(){
  return {
     restrict: 'A',
     require: 'select',
     link: function(scope, el, attrs, select){
       select.$optionChangeListeners.push(function selectListener(items){
          if (el.selectPlugin.initialized) {
            el.selectPlugin.refresh();
          } else {
            el.selectPlugin.init();
          }
       });
     }
  };
});
<select select-plugin ng-options="item.id as item.name for item in items | filter:filterFn"></select>

Closes #9222

if (!(length = selectCtrl.$optionChangeListeners.length)) return;

for(index = 0; index < length; index++) {
selectCtrl.$optionChangeListeners[index](values);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe pass in an empty array if values is not defined to be consistent? values || []

@caitp
Copy link
Contributor

caitp commented Sep 26, 2014

ultimately it's just a workaround for changes to DCCD, so we might come up with a better way to do this. Putting this on the backburner for now.

@pocesar
Copy link
Contributor Author

pocesar commented Sep 26, 2014

@caitp what is DCCD? I need this, and I hate to have to resort to a patched version instead an official release. On the other issue I thought it wouldn't be a waste of time doing this.

@caitp
Copy link
Contributor

caitp commented Sep 26, 2014

dirty checking change detection

@pocesar pocesar changed the title feat(selectDirective): add $optionChangeListeners to selectDirective controller feat(select): add $optionChangeListeners to selectDirective controller Sep 27, 2014
@IgorMinar
Copy link
Contributor

per #9222 (comment):

I suggest that you create a directive that will register a DOM mutation observer (DMO) on the select element and reexecute the jQuery plugin when DMO fires.

This particular callback is just a specialized version of a DMO callback. I don't think that we should add it because we would need to add dozens of similar callbacks throughout angular then whenever people need to know about angular changing the dom.

@IgorMinar IgorMinar closed this Sep 30, 2014
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.

$watch doesn't work the way it used to work (regression)
5 participants