fix(dropdown): Dedup code and handle differences if dropdown-menu used

Author: bleggett

Diff for: src/dropdown/dropdown.js

@@ -60,7 +60,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
     else if ( openScope.isKeynavEnabled() && /(38|40)/.test(evt.which) && openScope.isOpen ) {
       evt.preventDefault();
       evt.stopPropagation();
-      openScope.focusDropdownEntry(evt.which);
+      openScope.focusMenuEntry(evt.which);
     }
   };
 }])
@@ -71,7 +71,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
 	templateScope,
     openClass = dropdownConfig.openClass,
     getIsOpen,
-    setIsOpen = angular.noop,
+  setIsOpen = angular.noop,
     toggleInvoker = $attrs.onToggle ? $parse($attrs.onToggle) : angular.noop,
     appendToBody = false,
     keynavEnabled =false,
@@ -125,8 +125,8 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
     return keynavEnabled;
   };
 
-  scope.focusDropdownEntry = function(keyCode) {
-    var elems = self.dropdownMenu ? //If append to body is used.
+  scope.focusMenuEntry = function(keyCode) {
+    var elems = self.dropdownMenu ?
       (angular.element(self.dropdownMenu).find('a')) :
       (angular.element(self.$element).find('ul').eq(0).find('a'));
 
@@ -135,20 +135,12 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
         if ( !angular.isNumber(self.selectedOption)) {
           self.selectedOption = 0;
         } else {
-          self.selectedOption = (self.selectedOption === elems.length -1 ?
-                                 self.selectedOption :
-                                 self.selectedOption+1);
+          self.selectedOption = (self.selectedOption === elems.length -1 ? self.selectedOption : self.selectedOption+1);
         }
       }
       break;
       case (38): {
-        if ( !angular.isNumber(self.selectedOption)) {
-          return;
-        } else {
-          self.selectedOption = (self.selectedOption === 0 ?
-                                 0 :
-                                 self.selectedOption-1);
-        }
+        self.selectedOption = (self.selectedOption === 0 ? 0 : self.selectedOption-1);
       }
       break;
     }

Diff for: src/dropdown/test/dropdown.spec.js

@@ -483,131 +483,27 @@ describe('dropdownToggle', function() {
       triggerKeyDown($document, 40);
 
       expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var focusEl = element.find('ul').eq(0).find('a').eq(0);
-      expect(isFocused(focusEl)).toBe(true);
-    });
-
-    it('should not focus first list element when up arrow pressed after dropdown toggled', function() {
-      $document.find('body').append(element);
-      clickDropdownToggle();
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-
-      triggerKeyDown($document, 38);
-      var focusEl = element.find('ul').eq(0).find('a').eq(0);
-      expect(isFocused(focusEl)).toBe(false);
+      var optionEl = element.find('ul').eq(0).find('a').eq(0);
+      expect(isFocused(optionEl)).toBe(true);
     });
 
-    it('should not focus any list element when down arrow pressed if closed', function() {
+    it('should not focus first list element when down arrow pressed if closed', function() {
       $document.find('body').append(element);
       triggerKeyDown($document, 40);
 
       expect(element.hasClass(dropdownConfig.openClass)).toBe(false);
-      var focusEl = element.find('ul').eq(0).find('a');
-      expect(isFocused(focusEl[0])).toBe(false);
-      expect(isFocused(focusEl[1])).toBe(false);
-    });
-
-    it('should not change focus when other keys are pressed', function() {
-      $document.find('body').append(element);
-      clickDropdownToggle();
-      triggerKeyDown($document, 37);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var focusEl = element.find('ul').eq(0).find('a');
-      expect(isFocused(focusEl[0])).toBe(false);
-      expect(isFocused(focusEl[1])).toBe(false);
-    });
-
-    it('should focus second list element when down arrow pressed twice', function() {
-      $document.find('body').append(element);
-      clickDropdownToggle();
-      triggerKeyDown($document, 40);
-      triggerKeyDown($document, 40);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var focusEl = element.find('ul').eq(0).find('a').eq(1);
-      expect(isFocused(focusEl)).toBe(true);
-    });
-
-    it('should focus first list element when down arrow pressed 2x and up pressed 1x', function() {
-      $document.find('body').append(element);
-      clickDropdownToggle();
-      triggerKeyDown($document, 40);
-      triggerKeyDown($document, 40);
-
-      triggerKeyDown($document, 38);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var focusEl = element.find('ul').eq(0).find('a').eq(0);
-      expect(isFocused(focusEl)).toBe(true);
-    });
-
-    it('should stay focused on final list element if down pressed at list end', function() {
-      $document.find('body').append(element);
-      clickDropdownToggle();
-      triggerKeyDown($document, 40);
-      triggerKeyDown($document, 40);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var focusEl = element.find('ul').eq(0).find('a').eq(1);
-      expect(isFocused(focusEl)).toBe(true);
-
-      triggerKeyDown($document, 40);
-      expect(isFocused(focusEl)).toBe(true);
-    });
-
-    it('should close if esc is pressed while focused', function() {
-      element = dropdown('disabled');
-      $document.find('body').append(element);
-      clickDropdownToggle();
-
-      triggerKeyDown($document, 40);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
       var focusEl = element.find('ul').eq(0).find('a').eq(0);
-      expect(isFocused(focusEl)).toBe(true);
-
-      triggerKeyDown($document, 27);
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(false);
-    });
-  });
-
-  describe('`keyboard-nav` option with `dropdown-append-to-body` option', function() {
-    function dropdown() {
-      return $compile('<li dropdown dropdown-append-to-body keyboard-nav><a href dropdown-toggle></a><ul class="dropdown-menu" id="dropdown-menu"><li><a href>Hello On Body</a></li><li><a href>Hello Again</a></li></ul></li>')($rootScope);
-    }
-
-    beforeEach(function() {
-      element = dropdown();
-    });
-
-    it('should focus first list element when down arrow pressed', function() {
-      clickDropdownToggle();
-
-      triggerKeyDown($document, 40);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var focusEl = $document.find('ul').eq(0).find('a');
-      expect(isFocused(focusEl)).toBe(true);
-    });
-
-    it('should not focus first list element when down arrow pressed if closed', function() {
-      triggerKeyDown($document, 40);
-
-      expect(element.hasClass(dropdownConfig.openClass)).toBe(false);
-      var focusEl = $document.find('ul').eq(0).find('a');
       expect(isFocused(focusEl)).toBe(false);
     });
 
     it('should focus second list element when down arrow pressed twice', function() {
+      $document.find('body').append(element);
       clickDropdownToggle();
       triggerKeyDown($document, 40);
       triggerKeyDown($document, 40);
 
       expect(element.hasClass(dropdownConfig.openClass)).toBe(true);
-      var elem1 = $document.find('ul');
-      var elem2 = elem1.find('a');
-      var focusEl = $document.find('ul').eq(0).find('a').eq(1);
+      var focusEl = element.find('ul').eq(0).find('a').eq(1);
       expect(isFocused(focusEl)).toBe(true);
     });
   });