fix(select): don't register options when select has no ngModel #14864
Conversation
|
WRT "You can actually use ngOptions without ngModel to render a list of options". |
Afaict, this PR breaks these usecases. I think we should keep the current behaviors (e.g. set the |
|
Sorry, I don't follow. Why does this PR break these use cases? Do you mean simply using ngValue, interpolated value or text to set the value? The registerOption function should (and I am pretty sure does) only include code that integrates the options with the select controller, so listening on changes, removal etc. and then updating the interal representation, calling render, setViewValue etc, You are right, ngOptions must be used with ngModel. 😓 Using it without could be interesting for the material guys actually, as ng-options has been requested. |
Sorry, I was exaggerating a bit 😄 I meant it breaks the case with |
|
You mean because interpolateTextFn sets the value when the text changes? Since HTML5 uses the text value of an option as its value if there's no value attribute, I thought this would be okay. But if someone specifically listens on the value attribute of an option that has interpolated text, yes that would be a BC. But I think we can live that (the changes are in 1.6 only anyway) |
|
We actually used to not add options to the selectCtrl if there was no ngModel: angular.js/src/ng/directive/select.js Line 754 in bba6004 |
But then we used to handle the Anyway, since the select's value will be set correctly, I am fine dropping this as long as we mention the BC. |
|
Even that might be technically an implementation detail as the option directive is not documented at all. |
When option elements use ngValue, value or interpolated text to set the option value, i.e. when the parent select doesn't have an ngModel, there is no necessity in registering the options with the select controller. The registration was usually not a problem, as the ngModelController is set to a noop controller, so that all further interactions are aborted ($render etc) However, since f02b707 ngValue sets a hashed value inside the option value (to support arbitrary value types). This can cause issues with tests that expect unhashed values. The issue was found in angular-material, which uses select + ngValue to populate mdSelect. POSSIBLE BREAKING CHANGE: Option elements will no longer set their value attribute from their text value when their select element has no ngModel associated. Setting the value is only needed for the select directive to match model values and options. If no ngModel is present, the select directive doesn't need it. This should not affect many applications as the behavior was undocumented and not part of a public API. It also has no effect on the usual HTML5 behavior that sets the select value to the option text if the option does not provide a value attribute.
Narretz
force-pushed
the
fix-select-ngvalue
branch
from
7c37674 to
d851e8f
Compare
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
bug fix
What is the current behavior? (You can also link to an open issue here)
Options are added even if select has no ngModel. ngValue replaces the value attribute of the element with a hashed value, and this might break tests (it does in angular material)
What is the new behavior (if this is a feature change)?
Options are not added to select
Does this PR introduce a breaking change?
?
I can't think of a valid use case where you'd want registerning options without ngModel integration. If you don't have model binding, then you don't need to store the options internally. You can actually use ngOptions without ngModel to render a list of options - but you don't need to register the options for that.
The registerOption function is also not documented, so this is an implementation detail from the perspective of the user
Please check if the PR fulfills these requirements
Other information:
When option elements use ngValue, value or interpolated text simply to set
the option value, i.e. when the parent select doesn't have an ngModel,
there is no necessity in registering the options with the select controller.
This was previously no problem, as the ngModelController is set to a noop controller,
so that all further interactions are aborted ($render etc)
However, ngValue sets a hashed value inside the option value (to support arbitrary object types).
This can cause issues with tests that expect unhashed values.