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

Binding in select with multiple using optgroups fails #1553

Closed
cgross opened this issue Nov 10, 2012 · 4 comments
Closed

Binding in select with multiple using optgroups fails #1553

cgross opened this issue Nov 10, 2012 · 4 comments

Comments

@cgross
Copy link

cgross commented Nov 10, 2012

If I create a and use for groupings. The selected items aren't applied to the ng-model binding. Another person already created a fiddle demonstrating the issue: http://jsfiddle.net/laguiz/MDeme/1/

@petebacondarwin
Copy link
Contributor

Can you be clear what you expect it to do? In this fiddle, it looks like you are expecting multiple selections but the select is not multiple.

@lanterndev
Copy link
Contributor

@petebacondarwin I just hit this issue and created a fiddle demonstrating it: http://jsfiddle.net/YESMG/1/

is it clear now that this is a bug? if so, any pointers on how to fix?

not sure if @cgross is still interested in a fix, but I sure am. will update this issue with any progress.

@lanterndev
Copy link
Contributor

I dug into the select.js code and it is indeed buggy. Multiple is assuming selectElement.children() are all option elements, but they could also be optgroup elements. (I left a comment on one of the buggy lines here.)

The following patch (against angular.js 1.0.2) fixes the issue in the fiddle linked above:

diff --git a/app/lib/angular/angular.js b/app/lib/angular/angular.js
index ed50432..6d4465e 100644
--- a/app/lib/angular/angular.js
+++ b/app/lib/angular/angular.js
@@ -14101,10 +14101,16 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
         selectElement.bind('change', function() {
           scope.$apply(function() {
             var array = [];
-            forEach(selectElement.children(), function(option) {
+            function addIfSelected(option) {
               if (option.selected) {
                 array.push(option.value);
               }
+            }
+            forEach(selectElement.children(), function(child) {
+              if (child.tagName == 'OPTION')
+                addIfSelected(child);
+              else if (child.tagName == 'OPTGROUP')
+                forEach(child.getElementsByTagName('option'), addIfSelected);
             });
             ctrl.$setViewValue(array);
           });

Could someone pick up the ball and run with it from here to get a full fix applied?

lanterndev pushed a commit to getlantern/lantern-ui that referenced this issue Nov 29, 2012
lanterndev pushed a commit to getlantern/lantern-ui that referenced this issue Nov 29, 2012
@lanterndev
Copy link
Contributor

submitted pull request #1629

jbdeboer pushed a commit to jbdeboer/angular.js that referenced this issue Feb 9, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants