Dynamic option can hold empty value now so you don't have to initially p...

[slavede]

...ut it inside select (issue #11470)

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[slavede]

I signed it!

[googlebot]

CLAs look good, thanks!

[petebacondarwin]

@slavede - thanks for this PR. It looks good for the scenario of adding in a dynamic empty element. Isn't it great when a fix can be so simple. I am very pleased that you have added a nice unit test and signed the CLA. In the future you can also follow our git commit message guidelines to make your PRs even better!

I have two concerns that we need to resolve:

• what happens if there was already a static empty option when a dynamic empty option appears?
• what happens when a dynamic empty element is removed?
  • in the case that there was already a static empty option
  • in the case that there was no empty option already

[slavede]

Sorry, I was/am kinda busy this week. Let me think about this over the weekend and come up with some suggestion. Also, can you point me to git commit message guidelines?

[petebacondarwin]

Great! Here is the link you need https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit

[slavede]

1. that really shouldn't matter because self.emptyOptions is only used in checking if ngmodel is set to unknown value so we can set it to empty:

if (isUndefined(value) && self.emptyOption) {
    self.removeUnknownOption();
    $element.val('');
  } else {
    self.renderUnknownOption(value);
  }

2. tbh, currently self.emptyOption is not even checked in removeOption and if you now remove empty option, self.emptyOption would remain set so that's I believe another bug we could open/fix (or I can fix it within this bug fix?)

Proposed fix for 2:

@@ -98,6 +98,9 @@ var SelectController =
 if (count) {
   if (count === 1) {
     optionsMap.remove(value);
+        if (value === '') {
+          self.emptyOption = undefined;
+        }
   } else {
     optionsMap.put(value, count - 1);
   }

[slavede]

@petebacondarwin what do you think?

[petebacondarwin]

@slavede thanks for this. I tweaked the tests, added the bit about removing the empty option and then squashed the commits.