Prevent disabled options from being selected

docs/app/Examples/modules/Dropdown/index.js

@@ -43,12 +43,10 @@ const DropdownExamples = () => (
     <ExampleSection title='States'>
       <ComponentExample
         title='Disabled'
-        description='A disabled dropdown menu does not allow user interaction'
+        description='A disabled dropdown menu or item does not allow user interaction'
         examplePath='modules/Dropdown/States/Disabled'
       />
       <ComponentExample
-        title='Disabled Item'
-        description='A disabled dropdown item does not allow user interaction'
         examplePath='modules/Dropdown/States/DisabledItem'
       />
     </ExampleSection>

src/modules/Dropdown/Dropdown.js

@@ -519,7 +519,7 @@ export default class Dropdown extends Component {
     if (search && newQuery && !open) this.open()
 
     this.setState({
-      selectedIndex: 0,
+      selectedIndex: this.getEnabledIndices()[0],
       searchQuery: newQuery,
     })
   }
@@ -528,9 +528,11 @@ export default class Dropdown extends Component {
   // Getters
   // ----------------------------------------
 
-  getMenuOptions = () => {
+  // There are times when we need to calculate the options based on a value
+  // that hasn't yet been persisted to state.
+  getMenuOptions = (value = this.state.value) => {
     const { multiple, search, allowAdditions, additionPosition, additionLabel, options } = this.props
-    const { searchQuery, value } = this.state
+    const { searchQuery } = this.state
 
     let filteredOptions = options
 
@@ -565,6 +567,15 @@ export default class Dropdown extends Component {
     return _.get(options, `[${selectedIndex}]`)
   }
 
+  getEnabledIndices = (givenOptions) => {
+    const options = givenOptions || this.getMenuOptions()
+
+    return _.reduce(options, (memo, item, index) => {
+      if (!item.disabled) memo.push(index)
+      return memo
+    }, [])
+  }
+
   getItemByValue = (value) => {
     const { options } = this.props
     return _.find(options, { value })
@@ -585,27 +596,34 @@ export default class Dropdown extends Component {
     debug('value', value)
     const { multiple } = this.props
     const { selectedIndex } = this.state
-    const options = this.getMenuOptions()
+    const options = this.getMenuOptions(value)
+    const enabledIndicies = this.getEnabledIndices(options)
     const newState = {
       searchQuery: '',
     }
 
     // update the selected index
     if (!selectedIndex) {
+      const firstIndex = enabledIndicies[0]
+
       // Select the currently active item, if none, use the first item.
       // Multiple selects remove active items from the list,
       // their initial selected index should be 0.
-      newState.selectedIndex = multiple ? 0 : this.getMenuItemIndexByValue(value || _.get(options, '[0].value'))
+      newState.selectedIndex = multiple
+        ? firstIndex
+        : this.getMenuItemIndexByValue(value || _.get(options, `[${firstIndex}].value`))
     } else if (multiple) {
       // multiple selects remove options from the menu as they are made active
       // keep the selected index within range of the remaining items
       if (selectedIndex >= options.length - 1) {
-        newState.selectedIndex = selectedIndex - 1
+        newState.selectedIndex = enabledIndicies[enabledIndicies.length - 1]
       }
     } else {
+      const activeIndex = this.getMenuItemIndexByValue(value)
+
       // regular selects can only have one active item
       // set the selected index to the currently active item
-      newState.selectedIndex = this.getMenuItemIndexByValue(value)
+      newState.selectedIndex = _.includes(enabledIndicies, activeIndex) ? activeIndex : undefined
     }
 
     this.trySetState({ value }, newState)
@@ -626,10 +644,9 @@ export default class Dropdown extends Component {
     this.onChange(e, newValue)
   }
 
-  moveSelectionBy = (offset) => {
+  moveSelectionBy = (offset, startIndex = this.state.selectedIndex) => {
     debug('moveSelectionBy()')
     debug(`offset: ${offset}`)
-    const { selectedIndex } = this.state
 
     const options = this.getMenuOptions()
     const lastIndex = options.length - 1
@@ -639,14 +656,13 @@ export default class Dropdown extends Component {
 
     // next is after last, wrap to beginning
     // next is before first, wrap to end
-    let nextIndex = selectedIndex + offset
+    let nextIndex = startIndex + offset
     if (nextIndex > lastIndex) nextIndex = 0
     else if (nextIndex < 0) nextIndex = lastIndex
 
-    this.setState({ selectedIndex: nextIndex })
-
-    if (options[nextIndex].disabled) return this.moveSelectionBy(offset)
+    if (options[nextIndex].disabled) return this.moveSelectionBy(offset, nextIndex)
 
+    this.setState({ selectedIndex: nextIndex })
     this.scrollSelectedItemIntoView()
   }
 

test/specs/modules/Dropdown/Dropdown-test.js

@@ -115,6 +115,27 @@ describe('Dropdown Component', () => {
         .first()
         .should.have.prop('selected', true)
     })
+    it('defaults to the first non-disabled item', () => {
+      options[0].disabled = true
+      wrapperShallow(<Dropdown options={options} selection />)
+
+      // selection moved to second item
+      wrapper
+        .find('DropdownItem')
+        .first()
+        .should.have.prop('selected', false)
+
+      wrapper
+        .find('DropdownItem')
+        .at(1)
+        .should.have.prop('selected', true)
+    })
+    it('is null when all options disabled', () => {
+      const disabledOptions = options.map((o) => ({ ...o, disabled: true }))
+
+      wrapperRender(<Dropdown options={disabledOptions} selection />)
+        .should.not.have.descendants('.selected')
+    })
     it('is set when clicking an item', () => {
       // random item, skip the first as its selected by default
       const randomIndex = 1 + _.random(options.length - 2)
@@ -137,7 +158,7 @@ describe('Dropdown Component', () => {
         .find('DropdownItem')
         .at(randomIndex)
         .simulate('click', nativeEvent)
-        .should.have.not.prop('selected', true)
+        .should.not.have.prop('selected', true)
 
       dropdownMenuIsOpen()
     })
@@ -299,23 +320,6 @@ describe('Dropdown Component', () => {
 
       item.should.have.prop('active', true)
     })
-    it('does not become active on enter when disabled', () => {
-      const disabledOptions = _.map(options, (o) => ({ ...o, disabled: true }))
-      const item = wrapperMount(<Dropdown options={disabledOptions} selection />)
-        .simulate('click')
-        .find('DropdownItem')
-        .at(0)
-
-      // initial item props
-      item.should.have.prop('selected', true)
-      item.should.have.prop('active', false)
-
-      // attempt to make active
-      domEvent.keyDown(document, { key: 'Enter' })
-
-      item.should.have.prop('active', false)
-      dropdownMenuIsOpen()
-    })
     it('closes the menu', () => {
       wrapperMount(<Dropdown options={options} selection />)
         .simulate('click')