fix(Dropdown): compute proper selectedIndex in multiple (#4006)

Semantic-Org · Jul 29, 2020 · 50ec408 · 50ec408
1 parent 28eee26
commit 50ec408
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 6 deletions.
diff --git a/src/modules/Dropdown/utils/getSelectedIndex.js b/src/modules/Dropdown/utils/getSelectedIndex.js
@@ -27,7 +27,7 @@ export default function getSelectedIndex(config) {
     multiple,
     search,
   })
-  const enabledIndicies = _.reduce(
+  const enabledIndexes = _.reduce(
     menuOptions,
     (memo, item, index) => {
       if (!item.disabled) memo.push(index)
@@ -40,30 +40,32 @@ export default function getSelectedIndex(config) {
 
   // update the selected index
   if (!selectedIndex || selectedIndex < 0) {
-    const firstIndex = enabledIndicies[0]
+    const firstIndex = enabledIndexes[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.
     newSelectedIndex = multiple
       ? firstIndex
-      : _.findIndex(menuOptions, ['value', value]) || enabledIndicies[0]
+      : _.findIndex(menuOptions, ['value', value]) || enabledIndexes[0]
   } else if (multiple) {
+    newSelectedIndex = _.find(enabledIndexes, (index) => index >= selectedIndex)
+
     // multiple selects remove options from the menu as they are made active
     // keep the selected index within range of the remaining items
     if (selectedIndex >= menuOptions.length - 1) {
-      newSelectedIndex = enabledIndicies[enabledIndicies.length - 1]
+      newSelectedIndex = enabledIndexes[enabledIndexes.length - 1]
     }
   } else {
     const activeIndex = _.findIndex(menuOptions, ['value', value])
 
     // regular selects can only have one active item
     // set the selected index to the currently active item
-    newSelectedIndex = _.includes(enabledIndicies, activeIndex) ? activeIndex : undefined
+    newSelectedIndex = _.includes(enabledIndexes, activeIndex) ? activeIndex : undefined
   }
 
   if (!newSelectedIndex || newSelectedIndex < 0) {
-    newSelectedIndex = enabledIndicies[0]
+    newSelectedIndex = enabledIndexes[0]
   }
 
   return newSelectedIndex

diff --git a/test/specs/modules/Dropdown/Dropdown-test.js b/test/specs/modules/Dropdown/Dropdown-test.js
@@ -1521,6 +1521,37 @@ describe('Dropdown', () => {
       wrapper.should.have.exactly(options.length - 1).descendants('DropdownItem')
       wrapper.find('DropdownItem').last().should.have.prop('selected', true)
     })
+    it('keeps the selection on the same index', () => {
+      wrapperMount(<Dropdown options={options} selection multiple />)
+
+      wrapper.simulate('click')
+      dropdownMenuIsOpen()
+
+      wrapper.simulate('keydown', { key: 'ArrowDown' })
+      wrapper.find('DropdownItem').at(1).should.have.prop('selected', true)
+
+      wrapper.simulate('keydown', { key: 'Enter' })
+      wrapper.find('DropdownItem').at(1).should.have.prop('selected', true)
+    })
+    it('skips disabled items in selection', () => {
+      const testOptions = [
+        { value: 'foo', key: 'foo', text: 'foo' },
+        { value: 'bar', key: 'bar', text: 'bar' },
+        { value: 'baz', key: 'baz', text: 'baz', disabled: true },
+        { value: 'qux', key: 'qux', text: 'qux' },
+      ]
+
+      wrapperMount(<Dropdown options={testOptions} selection multiple />)
+
+      wrapper.simulate('click')
+      dropdownMenuIsOpen()
+
+      wrapper.simulate('keydown', { key: 'ArrowDown' })
+      wrapper.find('DropdownItem').at(1).should.have.prop('selected', true)
+
+      wrapper.simulate('keydown', { key: 'Enter' })
+      wrapper.find('DropdownItem').at(2).should.have.prop('selected', true)
+    })
     it('has labels with delete icons', () => {
       // add a value so we have a label
       const value = [_.head(options).value]