fix(Dropdown): stop onOpen from firing twice on icon click (Semantic-Org#2744)

* fix(Dropdown): stop onOpen from firing twice on icon click

* test(Dropdown): test onOpen and check it fires once with search prop

Author: willb335

src/modules/Dropdown/Dropdown.js

@@ -43,10 +43,7 @@ export default class Dropdown extends Component {
     as: customPropTypes.as,
 
     /** Label prefixed to an option added by a user. */
-    additionLabel: PropTypes.oneOfType([
-      PropTypes.element,
-      PropTypes.string,
-    ]),
+    additionLabel: PropTypes.oneOfType([PropTypes.element, PropTypes.string]),
 
     /** Position of the `Add: ...` option in the dropdown list ('top' or 'bottom'). */
     additionPosition: PropTypes.oneOf(['top', 'bottom']),
@@ -55,10 +52,7 @@ export default class Dropdown extends Component {
      * Allow user additions to the list of options (boolean).
      * Requires the use of `selection`, `options` and `search`.
      */
-    allowAdditions: customPropTypes.every([
-      customPropTypes.demand(['options', 'selection', 'search']),
-      PropTypes.bool,
-    ]),
+    allowAdditions: customPropTypes.every([customPropTypes.demand(['options', 'selection', 'search']), PropTypes.bool]),
 
     /** A Dropdown can reduce its complexity. */
     basic: PropTypes.bool,
@@ -69,10 +63,7 @@ export default class Dropdown extends Component {
     /** Primary content. */
     children: customPropTypes.every([
       customPropTypes.disallow(['options', 'selection']),
-      customPropTypes.givenProps(
-        { children: PropTypes.any.isRequired },
-        PropTypes.element.isRequired,
-      ),
+      customPropTypes.givenProps({ children: PropTypes.any.isRequired }, PropTypes.element.isRequired),
     ]),
 
     /** Additional classes. */
@@ -103,20 +94,14 @@ export default class Dropdown extends Component {
     /** Currently selected label in multi-select. */
     defaultSelectedLabel: customPropTypes.every([
       customPropTypes.demand(['multiple']),
-      PropTypes.oneOfType([
-        PropTypes.number,
-        PropTypes.string,
-      ]),
+      PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
     ]),
 
     /** Initial value or value array if multiple. */
     defaultValue: PropTypes.oneOfType([
       PropTypes.number,
       PropTypes.string,
-      PropTypes.arrayOf(PropTypes.oneOfType([
-        PropTypes.string,
-        PropTypes.number,
-      ])),
+      PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.number])),
     ]),
 
     /** A dropdown menu can open to the left or to the right. */
@@ -138,10 +123,7 @@ export default class Dropdown extends Component {
     header: PropTypes.node,
 
     /** Shorthand for Icon. */
-    icon: PropTypes.oneOfType([
-      PropTypes.node,
-      PropTypes.object,
-    ]),
+    icon: PropTypes.oneOfType([PropTypes.node, PropTypes.object]),
 
     /** A dropdown can be formatted to appear inline in other content. */
     inline: PropTypes.bool,
@@ -283,17 +265,10 @@ export default class Dropdown extends Component {
      * A selection dropdown can allow a user to search through a large list of choices.
      * Pass a function here to replace the default search.
      */
-    search: PropTypes.oneOfType([
-      PropTypes.bool,
-      PropTypes.func,
-    ]),
+    search: PropTypes.oneOfType([PropTypes.bool, PropTypes.func]),
 
     /** A shorthand for a search input. */
-    searchInput: PropTypes.oneOfType([
-      PropTypes.array,
-      PropTypes.node,
-      PropTypes.object,
-    ]),
+    searchInput: PropTypes.oneOfType([PropTypes.array, PropTypes.node, PropTypes.object]),
 
     /** Current value of searchQuery. Creates a controlled component. */
     searchQuery: PropTypes.string,
@@ -312,10 +287,7 @@ export default class Dropdown extends Component {
     /** Currently selected label in multi-select. */
     selectedLabel: customPropTypes.every([
       customPropTypes.demand(['multiple']),
-      PropTypes.oneOfType([
-        PropTypes.string,
-        PropTypes.number,
-      ]),
+      PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
     ]),
 
     /** A dropdown can be used to select between choices in a form. */
@@ -329,30 +301,20 @@ export default class Dropdown extends Component {
     simple: PropTypes.bool,
 
     /** A dropdown can receive focus. */
-    tabIndex: PropTypes.oneOfType([
-      PropTypes.number,
-      PropTypes.string,
-    ]),
+    tabIndex: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
 
     /** The text displayed in the dropdown, usually for the active item. */
     text: PropTypes.string,
 
     /** Custom element to trigger the menu to become visible. Takes place of 'text'. */
-    trigger: customPropTypes.every([
-      customPropTypes.disallow(['selection', 'text']),
-      PropTypes.node,
-    ]),
+    trigger: customPropTypes.every([customPropTypes.disallow(['selection', 'text']), PropTypes.node]),
 
     /** Current value or value array if multiple. Creates a controlled component. */
     value: PropTypes.oneOfType([
       PropTypes.bool,
       PropTypes.string,
       PropTypes.number,
-      PropTypes.arrayOf(PropTypes.oneOfType([
-        PropTypes.bool,
-        PropTypes.string,
-        PropTypes.number,
-      ])),
+      PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.bool, PropTypes.string, PropTypes.number])),
     ]),
 
     /** A dropdown can open upward. */
@@ -381,12 +343,7 @@ export default class Dropdown extends Component {
     wrapSelection: true,
   }
 
-  static autoControlledProps = [
-    'open',
-    'searchQuery',
-    'selectedLabel',
-    'value',
-  ]
+  static autoControlledProps = ['open', 'searchQuery', 'selectedLabel', 'value']
 
   static _meta = {
     name: 'Dropdown',
@@ -430,12 +387,12 @@ export default class Dropdown extends Component {
       if (hasValue && nextProps.multiple && !isNextValueArray) {
         console.error(
           'Dropdown `value` must be an array when `multiple` is set.' +
-          ` Received type: \`${Object.prototype.toString.call(nextProps.value)}\`.`,
+            ` Received type: \`${Object.prototype.toString.call(nextProps.value)}\`.`,
         )
       } else if (hasValue && !nextProps.multiple && isNextValueArray) {
         console.error(
           'Dropdown `value` must not be an array when `multiple` is not set.' +
-          ' Either set `multiple={true}` or use a string or number value.',
+            ' Either set `multiple={true}` or use a string or number value.',
         )
       }
     }
@@ -456,7 +413,8 @@ export default class Dropdown extends Component {
     return !shallowEqual(nextProps, this.props) || !shallowEqual(nextState, this.state)
   }
 
-  componentDidUpdate(prevProps, prevState) { // eslint-disable-line complexity
+  componentDidUpdate(prevProps, prevState) {
+    // eslint-disable-line complexity
     debug('componentDidUpdate()')
     debug('to state:', objectDiff(prevState, this.state))
 
@@ -465,7 +423,7 @@ export default class Dropdown extends Component {
       debug('dropdown focused')
       if (!this.isMouseDown) {
         const { minCharacters, openOnFocus, search } = this.props
-        const openable = !search || (search && minCharacters === 1)
+        const openable = !search || (search && minCharacters === 1 && !this.state.open)
 
         debug('mouse is not down, opening')
         if (openOnFocus && openable) this.open()
@@ -500,11 +458,7 @@ export default class Dropdown extends Component {
     } else if (prevState.open && !this.state.open) {
       debug('dropdown closed')
       this.handleClose()
-      eventStack.unsub('keydown', [
-        this.closeOnEscape,
-        this.moveSelectionOnKeyDown,
-        this.selectItemOnEnter,
-      ])
+      eventStack.unsub('keydown', [this.closeOnEscape, this.moveSelectionOnKeyDown, this.selectItemOnEnter])
       eventStack.unsub('click', this.closeOnDocumentClick)
       if (!this.state.focus) {
         eventStack.unsub('keydown', this.removeItemOnBackspace)
@@ -539,9 +493,7 @@ export default class Dropdown extends Component {
 
   closeOnChange = (e) => {
     const { closeOnChange, multiple } = this.props
-    const shouldClose = _.isUndefined(closeOnChange)
-      ? !multiple
-      : closeOnChange
+    const shouldClose = _.isUndefined(closeOnChange) ? !multiple : closeOnChange
 
     if (shouldClose) this.close(e)
   }
@@ -831,8 +783,7 @@ export default class Dropdown extends Component {
 
         const re = new RegExp(_.escapeRegExp(strippedQuery), 'i')
 
-        filteredOptions = _.filter(filteredOptions, opt =>
-          re.test(deburr ? _.deburr(opt.text) : opt.text))
+        filteredOptions = _.filter(filteredOptions, opt => re.test(deburr ? _.deburr(opt.text) : opt.text))
       }
     }
 
@@ -846,10 +797,7 @@ export default class Dropdown extends Component {
         key: 'addition',
         // by using an array, we can pass multiple elements, but when doing so
         // we must specify a `key` for React to know which one is which
-        text: [
-          additionLabelElement,
-          <b key='addition-query'>{searchQuery}</b>,
-        ],
+        text: [additionLabelElement, <b key='addition-query'>{searchQuery}</b>],
         value: searchQuery,
         className: 'addition',
         'data-additional': true,
@@ -871,10 +819,14 @@ export default class Dropdown extends Component {
   getEnabledIndices = (givenOptions) => {
     const options = givenOptions || this.getMenuOptions()
 
-    return _.reduce(options, (memo, item, index) => {
-      if (!item.disabled) memo.push(index)
-      return memo
-    }, [])
+    return _.reduce(
+      options,
+      (memo, item, index) => {
+        if (!item.disabled) memo.push(index)
+        return memo
+      },
+      [],
+    )
   }
 
   getItemByValue = (value) => {
@@ -944,9 +896,7 @@ export default class Dropdown extends Component {
       // 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
-        : this.getMenuItemIndexByValue(value, options) || enabledIndicies[0]
+      newSelectedIndex = multiple ? firstIndex : this.getMenuItemIndexByValue(value, options) || enabledIndicies[0]
     } 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
@@ -1096,12 +1046,13 @@ export default class Dropdown extends Component {
     debug(`menu: ${menu}`)
     debug(`item: ${item}`)
     const isOutOfUpperView = item.offsetTop < menu.scrollTop
-    const isOutOfLowerView = (item.offsetTop + item.clientHeight) > menu.scrollTop + menu.clientHeight
+    const isOutOfLowerView = item.offsetTop + item.clientHeight > menu.scrollTop + menu.clientHeight
 
     if (isOutOfUpperView) {
       menu.scrollTop = item.offsetTop
     } else if (isOutOfLowerView) {
-      menu.scrollTop = (item.offsetTop + item.clientHeight) - menu.clientHeight
+      // eslint-disable-next-line no-mixed-operators
+      menu.scrollTop = item.offsetTop + item.clientHeight - menu.clientHeight
     }
   }
 
@@ -1152,15 +1103,9 @@ export default class Dropdown extends Component {
   renderText = () => {
     const { multiple, placeholder, search, text } = this.props
     const { searchQuery, value, open } = this.state
-    const hasValue = multiple
-      ? !_.isEmpty(value)
-      : !_.isNil(value) && value !== ''
+    const hasValue = multiple ? !_.isEmpty(value) : !_.isNil(value) && value !== ''
 
-    const classes = cx(
-      placeholder && !hasValue && 'default',
-      'text',
-      search && searchQuery && 'filtered',
-    )
+    const classes = cx(placeholder && !hasValue && 'default', 'text', search && searchQuery && 'filtered')
     let _text = placeholder
     if (searchQuery) {
       _text = null
@@ -1172,21 +1117,27 @@ export default class Dropdown extends Component {
       _text = _.get(this.getItemByValue(value), 'text')
     }
 
-    return <div className={classes} role='alert' aria-live='polite'>{_text}</div>
+    return (
+      <div className={classes} role='alert' aria-live='polite'>
+        {_text}
+      </div>
+    )
   }
 
   renderSearchInput = () => {
     const { search, searchInput } = this.props
     const { searchQuery } = this.state
 
     if (!search) return null
-    return DropdownSearchInput.create(searchInput, { defaultProps: {
-      inputRef: this.handleSearchRef,
-      onChange: this.handleSearchChange,
-      style: { width: this.computeSearchInputWidth() },
-      tabIndex: this.computeSearchInputTabIndex(),
-      value: searchQuery,
-    } })
+    return DropdownSearchInput.create(searchInput, {
+      defaultProps: {
+        inputRef: this.handleSearchRef,
+        onChange: this.handleSearchChange,
+        style: { width: this.computeSearchInputWidth() },
+        tabIndex: this.computeSearchInputTabIndex(),
+        value: searchQuery,
+      },
+    })
   }
 
   renderSearchSizer = () => {
@@ -1218,10 +1169,7 @@ export default class Dropdown extends Component {
         value: item.value,
       }
 
-      return Label.create(
-        renderLabel(item, index, defaultProps),
-        { defaultProps },
-      )
+      return Label.create(renderLabel(item, index, defaultProps), { defaultProps })
     })
   }
 
@@ -1234,19 +1182,19 @@ export default class Dropdown extends Component {
       return <div className='message'>{noResultsMessage}</div>
     }
 
-    const isActive = multiple
-      ? optValue => _.includes(value, optValue)
-      : optValue => optValue === value
-
-    return _.map(options, (opt, i) => DropdownItem.create({
-      active: isActive(opt.value),
-      onClick: this.handleItemClick,
-      selected: selectedIndex === i,
-      ...opt,
-      key: getKeyOrValue(opt.key, opt.value),
-      // Needed for handling click events on disabled items
-      style: { ...opt.style, pointerEvents: 'all' },
-    }))
+    const isActive = multiple ? optValue => _.includes(value, optValue) : optValue => optValue === value
+
+    return _.map(options, (opt, i) =>
+      DropdownItem.create({
+        active: isActive(opt.value),
+        onClick: this.handleItemClick,
+        selected: selectedIndex === i,
+        ...opt,
+        key: getKeyOrValue(opt.key, opt.value),
+        // Needed for handling click events on disabled items
+        style: { ...opt.style, pointerEvents: 'all' },
+      }),
+    )
   }
 
   renderMenu = () => {
@@ -1257,11 +1205,7 @@ export default class Dropdown extends Component {
     // single menu child
     if (!childrenUtils.isNil(children)) {
       const menuChild = Children.only(children)
-      const className = cx(
-        direction,
-        useKeyOnly(open, 'visible'),
-        menuChild.props.className,
-      )
+      const className = cx(direction, useKeyOnly(open, 'visible'), menuChild.props.className)
 
       return cloneElement(menuChild, { className, ...ariaOptions })
     }