refactor(select): make MSIE behaviour consistent with other browsers #10044
Conversation
…tion` matcher By using a new matcher our tests become less brittle with respect to unimportant extra attributes.
Only changing the `<option>` text value is not enough to trigger a render change in IE. We need to explicit update the `label` attribute too. Closes angular#9621
|
@petebacondarwin I'd rather we used an arrangement more like this just so the behaviour is for sure consistent --- at the very least we need to run the test for both IE and not. I'm still not positive this solves the underlying issue, but since your test commit was first I'm guessing it works, so LGTM if it's changed to work for non-IE too. |
|
@caitp - The only reason I didn't do this for all browsers is that it is not needed and adds further DOM manipulation overhead for all the other browsers. As you demonstrate the fix works on all browsers but with non-IE it works without. So effectively it is an optimization to leave it out. Interestingly, we could in fact ditch using text and use label exclusively, since this is what the browsers read anyway - if not defined label falls back on text. So if we did that it would be consistent across browsers but I was wondering if people rely on the option's text being updated? What do you think? |
|
I'm not too worried about the perf, the perf is going to suck no matter what we do for large selects, and is going to be irrelevant no matter what we do for normal ones. I would prefer to not have behaviour change across browsers unless it's absolutely necessary. You could always just enable the test on all browsers instead and leave the fix as-is if you're that concerned about the overhead, but it's not going to make a huge difference |
Revisit #10042 to test suspicions