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

fix(select): update option if interpolated value attribute changes #12582

Closed
wants to merge 1 commit into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Aug 13, 2015

Previously, the option would only update if the fallback
(the interpolated option text) was used. Now the value attribute will
be observed if it contains an interpolation.

Closes #12005

selectCtrl.addOption(attr.value, element);
selectCtrl.ngModelCtrl.$render();
chromeHack(element);
// Either the value attribute or the text content is static
Copy link
Member

Choose a reason for hiding this comment

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

I think the comment would be more accurate as "Both the value attribute and the text content are static", right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, it's a bit obscure: If the attr is defined, we create an interpolation fn for it (to check if we should observe). We then skip everything related to the option text, because the value takes precedence over it. Which means, while the option value attr might not be interpolated, the option text could still be. Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

You are right; then both wordings are inaccurate. It should be more like:
Either the value attribute exists and is static or it doesn't exists and the text content is static
(which although more accurate sounds messy anyway - so I leave it up to you :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think just noting that the attr value is static should be enough then. The code isn't that big, it should be relatively easy to find out what exactly happens.

@Narretz Narretz modified the milestones: 1.4.5, 1.4.6 Aug 31, 2015
@Narretz Narretz force-pushed the fix-select-option branch 2 times, most recently from fca2008 to dadb278 Compare August 31, 2015 20:52
Narretz added a commit to Narretz/angular.js that referenced this pull request Aug 31, 2015
This is for options added without ngOptions.
Previously, an option with an interpolated value attribute would
not be updated if the binding changed, i.e. the select controller would
not recognize the changed option. Now the value attribute will
be observed if it contains an interpolation.

Closes angular#12005
Closes angular#12582
This is for options added without ngOptions.
Previously, an option with an interpolated value attribute would
not be updated if the binding changed, i.e. the select controller would
not recognize the changed option. Now the value attribute will
be observed if it contains an interpolation.

Closes angular#12005
Closes angular#12582
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.

None yet

3 participants