-
Notifications
You must be signed in to change notification settings - Fork 27.4k
feat(ngOptions): extract parsing into a controller #14522
Conversation
Moves the code for parsing `ng-options` expressions into a controller on the `ngOptions` directive. This allows us to create custom directives that require `ngOptions` and can access the options list. An example would be a directive that automatically selects one of the options if none are selected. closes angular#7888
On a high level, my only concern with this is that we are exposing too much of our (currently internal) implementation, which binds us to the current API (potentially for little gain). Still, if we decide to commit ourselves to it, we should at least document what that API is |
I agree that this exposes maybe too much of the internal implementation. If you only need the options list, why do you need to expose trackBy, getWatchables etc? |
Quite honestly I don't and would guess most people won't. This was more of a first run at this idea. I don't claim to even completely understand how this code works so I was going for the minimal amount of changes I could make to do this. I'm open to other approaches and I agree that a simpler more well thought out public API would make sense. Another thought I had was moving the options parsing into a service rather than a controller. It would be something similar to I'm open to any feedback or changes. Just let me know what you think. |
Just to clarify what the API "might" look like for a var parsedOptions = $optionsParser("option.id as option.description for option in items");
var value = parsedOptions.getValue($scope, locals);
var selectAs = parsedOptions.getSelectAs($scope, locals);
var viewValue = parsedOptions.getViewValue($scope, locals);
var trackBy = parsedOptions.getTrackBy($scope, locals);
var trackByValue = parsedOptions.getTrackByValue($scope, locals);
var display = parsedOptions.getDisplay($scope, locals);
var groupBy = parsedOptions.getGroupBy($scope, locals);
var disableWhen = parsedOptions.getDisableWhen($scope, locals);
var values = parsedOptions.getValues($scope, locals); My main goal of this whole thing is to avoid need to open the source code to ngOptions, copy the huge regular expression, and paste it into my application code just so I can read the options. This is essentially what Angular Strap has done: https://github.com/mgcrea/angular-strap/blob/master/src/helpers/parse-options.js |
But do you really need all that info / all these methods? I wonder if the return value of this fn would be enough if it was exposed (surface area is smaller): https://github.com/angular/angular.js/blob/master/src/ng/directive/ngOptions.js#L364-L388 |
@klieber can you clarify which information you actually need? |
I'm going to close this issue because we haven't got any feedback and can't move forward / decide without it. Feel free to reopen this issue if you can provide new feedback. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behavior? (You can also link to an open issue here)
The logic for parsing
ng-options
is unavailable via a public api.What is the new behavior (if this is a feature change)?
Extracts the code for parsing the
ng-options
expression into a controller on thengOptions
directive so that custom directives can read the options.Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information:
Moves the code for parsing
ng-options
expressions into a controlleron the
ngOptions
directive. This allows us to create customdirectives that require
ngOptions
and can access the options list. Anexample would be a directive that automatically selects one of the options
if none are selected.
closes #7888