Dynamic select directive doesn't support empty value

[slavede]

Branch 1.2.X issue.

In case I have select directive which get options generated from server, it throws following error:

    TypeError: 'undefined' is not an object (evaluating 'emptyOption.prop')
        at /angularjs/angular-1.2.28.js:21717
        at ngModelWatch (/angularjs/angular-1.2.28.js:17899)
        at /angularjs/angular-1.2.28.js:12642
        at /angularjs/angular-1.2.28.js:12915

in case I'm trying to update model in my test (it works perfectly fine when it comes to browser, but it can't update itself based on model change) and here's why. This piece of code executes when linking select initially:

  for (var i = 0, children = element.children(), ii = children.length; i < ii; i++) {
    if (children[i].value === '') {
      emptyOption = nullOption = children.eq(i);
      break;
    }
  }

and in this moment of course, my select doesn't have emptyOption. Later on, I populate options and try to update model in my test. Then this piece of code is triggered:

      var viewValue = ngModelCtrl.$viewValue;

      if (selectCtrl.hasOption(viewValue)) {
        if (unknownOption.parent()) unknownOption.remove();
        selectElement.val(viewValue);
        if (viewValue === '') emptyOption.prop('selected', true); // to make IE9 happy
      } else {
        if (isUndefined(viewValue) && emptyOption) {
          selectElement.val('');
        } else {
          selectCtrl.renderUnknownOption(viewValue);
        }
      }

Now, in this moment emptyOption is undefined, but I really do have it, only thing, it was generated later (after link function).

Can we(I can do it if you let me) have additional element.find when it comes to assigning to emptyOption?

[petebacondarwin]

So your issue is that the select directive does not allow you to specify the empty option asynchronously. Correct?

[slavede]

Yes, that's right...

[petebacondarwin]

So the design of this directive is that the empty option should be hard-coded - even if the rest of the options are loaded from the server. Could you try doing that as a workaround?

[slavede]

Unfortunately no, because even display for this empty value should has to come from server. I have simple ng-repeat that iterates options I get from server and display can be different depending on place this directive is used.

[slavede]

If you agree, we can do in this line additional check if emptyOption is not defined, try to search for it:

if (viewValue === '') emptyOption.prop('selected', true); // to make IE9 happy

I would search it in the way initial search is done:

for (var i = 0, children = element.children(), ii = children.length; i < ii; i++) {
    if (children[i].value === '') {
       emptyOption = nullOption = children.eq(i);
       break;
    }
}

It would be a pleasure to do pull request on this...

[petebacondarwin]

I would rather not search every time. Perhaps we could come up with a solution that allows a new "empty" option to register itself when it arrives...

[slavede]

In that case you will have two empty options :) The problem here is that emptyOption reference is not defined in the moment we would use it. If you see, it enters this part of condition by checking:

if (selectCtrl.hasOption(viewValue)) {

so it's already there. Only thing, it's not bind to emptyOption variable causing it to throw exception.

[slavede]

OK, here is suggested fix. In addOption, which is being called each time option is created, we can check if value is '' and store that into self.emptyOption? Quick fix, what do you think?

Did I forget to mention, it's branch 1.2.X.

[petebacondarwin]

That could work. Do you want to try to create a Pull Request, with tests and docs?

[slavede]

Yeah sure. I'll do it, will take a look tomorrow

[slavede]

I did pull request, can you review? Thanks!

[slavede]

Hi @petebacondarwin , is it maybe something wrong with my pull request? If so, can you tell me what it is and I'll change it? Thanks