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

Dynamic select directive doesn't support empty value #11470

Closed
slavede opened this issue Mar 31, 2015 · 14 comments
Closed

Dynamic select directive doesn't support empty value #11470

slavede opened this issue Mar 31, 2015 · 14 comments

Comments

@slavede
Copy link
Contributor

slavede commented Mar 31, 2015

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?

@slavede slavede changed the title Dynamic select directive doesn't support empty valu Dynamic select directive doesn't support empty value Mar 31, 2015
@petebacondarwin
Copy link
Member

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

@slavede
Copy link
Contributor Author

slavede commented Apr 2, 2015

Yes, that's right...

@petebacondarwin
Copy link
Member

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
Copy link
Contributor Author

slavede commented Apr 3, 2015

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
Copy link
Contributor Author

slavede commented Apr 3, 2015

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
Copy link
Member

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
Copy link
Contributor Author

slavede commented Apr 3, 2015

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
Copy link
Contributor Author

slavede commented Apr 3, 2015

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
Copy link
Member

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

@slavede
Copy link
Contributor Author

slavede commented Apr 3, 2015

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

@slavede
Copy link
Contributor Author

slavede commented Apr 7, 2015

I did pull request, can you review? Thanks!

@slavede
Copy link
Contributor Author

slavede commented Apr 10, 2015

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

@petebacondarwin
Copy link
Member

Sorry I will take a look this week.

@slavede
Copy link
Contributor Author

slavede commented Apr 10, 2015

Thanks!

petebacondarwin pushed a commit that referenced this issue Apr 22, 2015
netman92 pushed a commit to netman92/angular.js that referenced this issue Aug 8, 2015
The select directive supports provision of an "empty" element that is used
if the value of the select is undefined.

This fix ensures that this empty option can be provided dynamically after
the initial compilation has completed.

Closes angular#11470
Closes angular#11512
ggershoni pushed a commit to ggershoni/angular.js that referenced this issue Sep 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants