fix(ngSelect): Support static options on multi-select drop down #5938
Conversation
Removes a condition where static options are not added as part of render function when multiple attribute is enabled Closes angular#4325
| } else if (!selectedSet) { | ||
| // option could not be found, we have to insert the undefined item | ||
| optionGroups[''].unshift({id:'?', label:'', selected:true}); | ||
| } |
There was a problem hiding this comment.
this looks like it's going to fail jshint/tests
There was a problem hiding this comment.
I didn't see any failure on "grunt test"
There was a problem hiding this comment.
I ran grunt jshint & its all green. Am I missing something?
There was a problem hiding this comment.
from the diff, it looks like there should be an unexpected closing brace. if not then ignore the comment, but the diff looks like it. Travis is being a bit slow right now, but we'll know soon enough :)
There was a problem hiding this comment.
just took a second look at this diff and now feels stupid for the above comments
derp
|
@caitp try diff excluding whitespaces: siddii@db96f77?w=1 |
|
@siddii could you explain clearly what this is actually solving? I'm going to put this down under 1.3.x but I feel like some info is needed. If this is fixing a proper bug with some real impact I'll try to squeeze it in earlier Actually, it sounds like this is probably good for 1.2.10... so, I'll see if we can do that |
|
@caitp - There was some assumption made in the code where in multi-select dropdown ( And also, this plunkr - http://plnkr.co/edit/jDijgjJ0EZLFUGIkTKDs?p=preview proves the expected behavior after the fix. Let me know if you have further questions. Thanks! |
|
I suspect that this code you are removing was fixing some old IE bug. |
…ions Shows the static option for a not selected value in a select with ngOption also when the multiple attribute is used. Closes angular#4325 Closes angular#5938
|
Hi @siddii, In a drop down box, you need an additional element to unselect a previously selected value. So why do you need that additional entry, what is your use case? |
|
Closing this for now. Please comment if you have a usecase that I overlooked. |
ghost
mentioned this pull request
Removes a condition where static options are not added as part of render function when multiple attribute is enabled
Closes #4325