Fixed an issue where you could click on optgroup label and break the plugin

Author: lcdsantos

public/jquery.selectric.js

@@ -952,19 +952,16 @@
 
       _this.utils.triggerCallback('BeforeHighlight', _this);
 
-      // Parameter index is required
-      if ( index === undefined ) {
+      // Parameter index is required and should not be a disabled item
+      if ( index === undefined || index === -1 || _this.lookupItems[index].disabled ) {
         return;
       }
 
-      // If element is disabled, can't select it
-      if ( index !== -1 && !_this.lookupItems[index].disabled ) {
-        $filteredLi
-          .eq(_this.state.highlightedIdx = index)
-          .addClass('highlighted');
+      $filteredLi
+        .eq(_this.state.highlightedIdx = index)
+        .addClass('highlighted');
 
-        _this.detectItemVisibility(index);
-      }
+      _this.detectItemVisibility(index);
 
       _this.utils.triggerCallback('Highlight', _this);
     },
@@ -980,9 +977,9 @@
 
       _this.utils.triggerCallback('BeforeSelect', _this, index);
 
-      // Don't select disabled items
-      if (index !== -1 && _this.lookupItems[index].disabled) {
-        return false;
+      // Parameter index is required and should not be a disabled item
+      if ( index === undefined || index === -1 || _this.lookupItems[index].disabled ) {
+        return;
       }
 
       if ( _this.state.multiple ) {

public/jquery.selectric.min.js

@@ -935,19 +935,16 @@
 
       _this.utils.triggerCallback('BeforeHighlight', _this);
 
-      // Parameter index is required
-      if ( index === undefined ) {
+      // Parameter index is required and should not be a disabled item
+      if ( index === undefined || index === -1 || _this.lookupItems[index].disabled ) {
         return;
       }
 
-      // If element is disabled, can't select it
-      if ( index !== -1 && !_this.lookupItems[index].disabled ) {
-        $filteredLi
-          .eq(_this.state.highlightedIdx = index)
-          .addClass('highlighted');
+      $filteredLi
+        .eq(_this.state.highlightedIdx = index)
+        .addClass('highlighted');
 
-        _this.detectItemVisibility(index);
-      }
+      _this.detectItemVisibility(index);
 
       _this.utils.triggerCallback('Highlight', _this);
     },
@@ -963,9 +960,9 @@
 
       _this.utils.triggerCallback('BeforeSelect', _this, index);
 
-      // Don't select disabled items
-      if (index !== -1 && _this.lookupItems[index].disabled) {
-        return false;
+      // Parameter index is required and should not be a disabled item
+      if ( index === undefined || index === -1 || _this.lookupItems[index].disabled ) {
+        return;
       }
 
       if ( _this.state.multiple ) {

src/jquery.selectric.js

@@ -58,6 +58,17 @@ describe('basic suite', function() {
     expect($('.selectric-wrapper').find('.selectric-group-label').length).toBe(3);
   });
 
+  it('should not select <optgroup> label', function() {
+    loadFixtures('optgroup.html');
+
+    select = $('#optgroup');
+    select.selectric();
+
+    $('.selectric').click();
+    $('.selectric-wrapper').find('.selectric-group-label').click();
+    expect(select.val()).toBe('0');
+  });
+
   it('should add class on hover', function() {
     $('.selectric').trigger('mouseenter');
     expect($('.selectric-wrapper').hasClass('selectric-hover')).toBe(true);
@@ -120,6 +131,14 @@ describe('basic suite', function() {
     expect($('.selectric-items').find('li:eq(1)').hasClass('disabled')).toBe(true);
   });
 
+  it('should not select disabled item', function() {
+    select.find('option:eq(2)').prop('disabled', true);
+    select.selectric('refresh');
+    $('.selectric').click();
+    $('.selectric-items').find('li:eq(2)').click();
+    expect(select.val()).toBe('apricot');
+  });
+
   it('should update label', function() {
     $('.selectric').click();
     $('.selectric-items').find('li:eq(4)').click();

test/basic.spec.js

@@ -1,5 +1,5 @@
 <select id="optgroup">
-  <option>Select with optgroup</option>
+  <option value="0">Select with optgroup</option>
   <optgroup label="Group 1">
     <option>Option 1.1</option>
   </optgroup>