feat(ngOptions): add support for disabling an option #11017

sjbarker · 2015-02-09T21:53:59Z

This patch adds support for disabling options based on model values. The
"disable when" syntax allows for listening to changes on those model values,
in order to dynamically enable and disable the options.

The changes prevent disabled options from being written to the selectCtrl
from the model. If a disabled selection is present on the model, normal
unknown or empty functionality kicks in.

closes #638

sjbarker · 2015-02-09T22:12:33Z

Explanation

The changes in this pull request reflect a long-time reoccurring and widely popular request for the ability to disable options dynamically in ngOptions. This solution accomplishes this feature by adding the disable when syntax to the ng-options directive. All current functionality is retained in select controls.

The solution for model changes reflecting disabled options is to keep the current functionality of "unknown" or "empty" options. Tests have been written to reflect this feature. Consider the following:

scope.selected = '';
scope.options = [ 'a', 'b', 'c', 'd'];
scope.disableOption = function(option) {
  return option === 'b';
}

<select ng-model="selected"
             ng-options="o disable when disableOption(o) for o in options"></select>

-OR-

scope.selected = '';
scope.options = [ 
  {
    name: 'a',
    disabled: false
  },
  {
    name: 'b',
    disabled: false
  },
  {
    name: 'c',
    disabled: false
  },
  {
    name: 'd',
    disabled: false
  }
];

<select ng-model="selected"
             ng-options="o.name disable when o.disabled for o in options"></select>

In either case, if selected becomes a disabled option, then select will render the empty or unknown option. This concurrent with any unknown or invalid option.

Current Issues

Issue #638 has been open for over three years now and is currently on backlog.

Current PR's

There is one current PR (#9854) by @cades that has been rendered obsolete by @petebacondarwin's select.js refactor (408f89d).

sjbarker · 2015-02-09T22:14:41Z

@petebacondarwin - Since you recently did the select directive refactor, will you take a quick look at this and pass it around? We have been sitting on this for a really long time and this would provide the functionality without breaking anything; and with room to grow in the future. Thanks!

cades · 2015-02-10T00:46:32Z

Great job, thanks to your effort and time!
Hope to merge this PR as soon as possible.

sjbarker · 2015-02-10T01:43:01Z

Thanks @cades. You were there first until they refactored the select directive. And thank you for your time and effort as well.

sjbarker · 2015-02-10T22:14:48Z

@pkozlowski-opensource - I figured I would tag you in this since you were attending to the #9854 PR. When you have a moment, will you take a look as well? Thanks.

petebacondarwin · 2015-02-12T19:04:25Z

OK, I like this idea, although I think that the syntax sound better with disable when rather than disable by.

Also I am concerned about add to the getWatchables array, which is already a bit of a pain point.

I realised that we can actually optimize this by only including things that are being specified. So for instance, if no disable when clause was provided we would not need to watch this expression. The same for the displayFn which computes the label.

sjbarker · 2015-02-13T02:06:55Z

No problem, @petebacondarwin. Let me get those changes in real quick. I will open an issue for optimizing the getWatchables array and cover it in this commit as well.

sjbarker · 2015-02-13T03:28:38Z

src/ng/directive/ngOptions.js

-                         //000011111111110000000000022222222220000000000000000000003333333333000000000000004444444444444440000000005555555555555550000000666666666666666000000000000000777777777700000000000000000008888888888
-var NG_OPTIONS_REGEXP = /^\s*([\s\S]+?)(?:\s+as\s+([\s\S]+?))?(?:\s+group\s+by\s+([\s\S]+?))?\s+for\s+(?:([\$\w][\$\w]*)|(?:\(\s*([\$\w][\$\w]*)\s*,\s*([\$\w][\$\w]*)\s*\)))\s+in\s+([\s\S]+?)(?:\s+track\s+by\s+([\s\S]+?))?$/;
+//                     //00001111111111000000000002222222222000000000000000000000333333333300000000000000000000000004444444444400000000000005555555555555550000000006666666666666660000000777777777777777000000000000000888888888800000000000000000009999999999
+var NG_OPTIONS_REGEXP = /^\s*([\s\S]+?)(?:\s+as\s+([\s\S]+?))?(?:\s+group\s+by\s+([\s\S]+?))?(?:\s+disable\s+when\s+([\s\S]+?))?\s+for\s+(?:([\$\w][\$\w]*)|(?:\(\s*([\$\w][\$\w]*)\s*,\s*([\$\w][\$\w]*)\s*\)))\s+in\s+([\s\S]+?)(?:\s+track\s+by\s+([\s\S]+?))?$/;


@petebacondarwin Here is the syntax change.

sjbarker · 2015-02-13T15:34:18Z

src/ng/directive/ngOptions.js

+          if (match[4]) {
+            var disableWhen = disableWhenFn(scope, locals);
+            watchedArray.push(disableWhen);
+          }


@petebacondarwin - I rebased your perf and mimicked it with the disableWhenFn

petebacondarwin · 2015-02-18T08:32:47Z

src/ng/directive/ngOptions.js

@@ -326,7 +350,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
  return {
    restrict: 'A',
    terminal: true,
-    require: ['select', '?ngModel'],
+    require: ['select', 'ngModel'],


ngModel is still optional here.

This patch adds support for disabling options based on model values. The "disable when" syntax allows for listening to changes on those model values, in order to dynamically enable and disable the options. The changes prevent disabled options from being written to the selectCtrl from the model. If a disabled selection is present on the model, normal unknown or empty functionality kicks in. closes angular#638

sjbarker · 2015-02-18T13:12:36Z

@petebacondarwin I returned the ngModel requirement to optional.

petebacondarwin · 2015-02-18T13:15:45Z

I added a few extra tests and merged into master - thanks @sjbarker and @cades !

sjbarker · 2015-02-18T13:19:24Z

Thanks @petebacondarwin! I didn't see that ngModel optional requirement make it in though. Is that ok?

petebacondarwin · 2015-02-18T13:23:19Z

Oops

On 18 February 2015 at 13:21, Stephen Barker notifications@github.com
wrote:

Thanks @petebacondarwin https://github.com/petebacondarwin! I didn't
see that ngModel optional requirement make it in though. Is that ok?

—
Reply to this email directly or view it on GitHub
#11017 (comment).

This patch adds support for disabling options based on model values. The "disable when" syntax allows for listening to changes on those model values, in order to dynamically enable and disable the options. The changes prevent disabled options from being written to the selectCtrl from the model. If a disabled selection is present on the model, normal unknown or empty functionality kicks in. Closes angular#638 Closes angular#11017

sjbarker · 2015-02-18T13:30:21Z

@petebacondarwin - f3ae91d here has the commit you just merged with the ngModel require change.

petebacondarwin · 2015-02-18T13:45:41Z

Thanks - I fixed that here: ef894c8

sjbarker · 2015-02-18T13:52:32Z

Ok perfect

IngoVals · 2015-06-15T16:01:42Z

If the disabling requirements are dynamic and something that was selected becomes disable the value is set to null, could we intercept this event and change it to something else?

sjbarker · 2015-06-18T01:54:30Z

@IngoVals when you say change it to something else, what do you mean?

This patch adds support for disabling options based on model values. The "disable when" syntax allows for listening to changes on those model values, in order to dynamically enable and disable the options. The changes prevent disabled options from being written to the selectCtrl from the model. If a disabled selection is present on the model, normal unknown or empty functionality kicks in. Closes angular#638 Closes angular#11017

googlebot added the cla: yes label Feb 9, 2015

sjbarker force-pushed the ng-options-disable-by branch from 77ae42d to 6237b45 Compare February 10, 2015 15:32

sjbarker force-pushed the ng-options-disable-by branch 2 times, most recently from 5afa825 to 6e468c8 Compare February 11, 2015 15:48

petebacondarwin added this to the 1.4.0-beta.6 / 1.3.15 milestone Feb 12, 2015

petebacondarwin self-assigned this Feb 12, 2015

petebacondarwin added type: feature branch: 1.4.x component: forms labels Feb 12, 2015

sjbarker mentioned this pull request Feb 13, 2015

refactor(ngOptions): optimize getWatchables array #11050

Closed

sjbarker force-pushed the ng-options-disable-by branch from 6e468c8 to e0c1ad7 Compare February 13, 2015 03:03

sjbarker reviewed Feb 13, 2015
View reviewed changes

petebacondarwin mentioned this pull request Feb 13, 2015

feat(select): ngOptions support disabling options #9854

Closed

sjbarker force-pushed the ng-options-disable-by branch from e0c1ad7 to ff55791 Compare February 13, 2015 15:30

sjbarker reviewed Feb 13, 2015
View reviewed changes

sjbarker force-pushed the ng-options-disable-by branch from ff55791 to dd1e6b8 Compare February 17, 2015 14:06

petebacondarwin reviewed Feb 18, 2015
View reviewed changes

sjbarker force-pushed the ng-options-disable-by branch from dd1e6b8 to 518bf01 Compare February 18, 2015 13:07

petebacondarwin closed this in da9eac8 Feb 18, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ngOptions): add support for disabling an option #11017

feat(ngOptions): add support for disabling an option #11017

sjbarker commented Feb 9, 2015

sjbarker commented Feb 9, 2015

sjbarker commented Feb 9, 2015

cades commented Feb 10, 2015

sjbarker commented Feb 10, 2015

sjbarker commented Feb 10, 2015

petebacondarwin commented Feb 12, 2015

sjbarker commented Feb 13, 2015

sjbarker Feb 13, 2015

sjbarker Feb 13, 2015

petebacondarwin Feb 18, 2015

sjbarker commented Feb 18, 2015

petebacondarwin commented Feb 18, 2015

sjbarker commented Feb 18, 2015

petebacondarwin commented Feb 18, 2015

sjbarker commented Feb 18, 2015

petebacondarwin commented Feb 18, 2015

sjbarker commented Feb 18, 2015

IngoVals commented Jun 15, 2015

sjbarker commented Jun 18, 2015

feat(ngOptions): add support for disabling an option #11017

feat(ngOptions): add support for disabling an option #11017

Conversation

sjbarker commented Feb 9, 2015

sjbarker commented Feb 9, 2015

Explanation

Current Issues

Current PR's

sjbarker commented Feb 9, 2015

cades commented Feb 10, 2015

sjbarker commented Feb 10, 2015

sjbarker commented Feb 10, 2015

petebacondarwin commented Feb 12, 2015

sjbarker commented Feb 13, 2015

sjbarker Feb 13, 2015

Choose a reason for hiding this comment

sjbarker Feb 13, 2015

Choose a reason for hiding this comment

petebacondarwin Feb 18, 2015

Choose a reason for hiding this comment

sjbarker commented Feb 18, 2015

petebacondarwin commented Feb 18, 2015

sjbarker commented Feb 18, 2015

petebacondarwin commented Feb 18, 2015

sjbarker commented Feb 18, 2015

petebacondarwin commented Feb 18, 2015

sjbarker commented Feb 18, 2015

IngoVals commented Jun 15, 2015

sjbarker commented Jun 18, 2015