f[eat|ix](select): support values added by ngValue

[Narretz]

select elements with ngModel will now set ngModel to option values added by ngValue.
This allows setting values of any type without the use of ngOptions.

Interpolations inside attributes can only be strings, but the ngValue directive uses attrs.$set,
which does not follow any type restriction. Any $observe on the value attribute will therefore receive
the original value (result of ngValue expression). However, when a user selects an option, the browser
sets the select value to the actual option's value attribute, which is still always a string.
For that reason, when option are added by ngValue, we set the hashed value of the original value in
the value attribute and store the actual value in an extra map. When the select value changes, we
read access the actual value via the hashed select value.

Closes #9842
Closes #6297

• Test that it takes precedence over interpolated option / option text
• Test for multiple selections
• Fix failing Chrome tests
• Docs

Talking points:

• The implementation works reasonably well, although it's a bit hackyspecial
• With ngValue, option values are hash values, not the real values (which makes sense, but might be confusing)
• afaik angular 2 suffers from the same problem as ng1 right now

[Narretz]

I wanted to fix this, but I'm not sure it's necessary. I don't think browsers have to set selected when the value of the select changes. It's strange though that is set before changing the select value via model update.

[petebacondarwin]

This test fails:

      it('should work update the value as a result of changes to the options', function() {
        var A = { name: 'A'}, B = { name: 'B'}, C = { name: 'C'};
        scope.options = [A, B, C];
        scope.obj = {};
        compile(
          '<select ng-model="obj.value">' +
            '<option ng-repeat="option in options" ng-value="option">{{$index}}</option>' +
          '</select>'
        );

        var optionElements = element.find('option');
        expect(optionElements.length).toEqual(4);
        browserTrigger(optionElements.eq(0));

        optionElements = element.find('option');
        expect(optionElements.length).toEqual(3);
        expect(scope.obj.value).toBe(A);

        scope.options.shift();
        scope.$digest();

        optionElements = element.find('option');
        expect(optionElements.length).toEqual(3);
        expect(scope.obj.value).toBe(null);
      });

[Narretz]

Have you tried if that works with regular interpolated options? Because I don't think it does

[petebacondarwin]

It certainly works with ngOptions, which is what we are trying to emulate, right?

[Narretz]

@petebacondarwin I've updated the PR again with the changes we talked about (support for disabled options, and model updates when the options change / are removed / get disabled / get added).

[petebacondarwin]

This pattern of running something once in the postDigest keeps appearing. I wonder if we should abstract it out into $scope?

[petebacondarwin]

I think you might have nailed it but it is the kind of change that I want to spend some time thinking about corner cases...

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

[petebacondarwin]

I did author some of these commits and I am OK with that :-)

[petebacondarwin]

no longer needed?

[Narretz]

I'll change the tests and then add some docs. Should I mentiond that performance in IE is going to be very bad with 3000+ options?

[petebacondarwin]

Why not

[Narretz]

While fixing up the PR, I found a behavior in ngOptions and select that I find a bit confusing: You have a select that is multiple and the model contains two values, one of which is matched by an option, while the other isn't.
What happens is that the matching option gets selected, and the model keeps both values. Imo, I would expect no option to be selected, as the model isn't completely matched by the select options. Thoughts?

[Narretz]

So in #13172 we decided that if no option at all can be selected, then the select should be invalid (required).

Doesn't say anything about a partial mismatch, though. We could decide not to select anything when not all options are matched, but I'm not convinced that this is a good idea.

Theoretically, setting the model to match the available options is the developers responsibility, so if that happens it's your own fault. However, it's not very easy to check if a select's options will match the model. We could expose the selectCtrl.hasOption function and make it work with ngOptions, so that users could manually check if a specific $viewValue is matched by their select, see (#12915)

[Narretz]

I'm also fine with making a decision on this later. As far as I can tell, the described behavior hasn't bothered anyone yet.
@petebacondarwin I need your perf commit as part of the commit that handles multiple selects correctly. Because otherwise, the current behavior doesn't work as expected. If the model has two values set, but only one is matched by options, then without the "perf" fix, the option will set the model to the one value only. Delaying the update after render handles this.

[gkalpak]

✨ 🎉 ✨