feat: use onClick handler on items rather than on root (#192)

src/__tests__/__snapshots__/downshift.get-root-props.js.snap

@@ -1,7 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`not applying the onClick prop results in an error 1`] = `"downshift: You must apply the \\"onClick\\" prop from getRootProps onto your root element."`;
-
 exports[`not applying the ref prop results in an error 1`] = `"downshift: You must apply the ref prop \\"ref\\" from getRootProps onto your root element."`;
 
 exports[`returning a DOM element and calling getRootProps with a refKey results in an error 1`] = `"downshift: You returned a DOM element. You should not specify a refKey in getRootProps. You specified \\"blah\\""`;

src/__tests__/downshift.get-root-props.js

@@ -47,18 +47,6 @@ test('not applying the ref prop results in an error', () => {
   expect(() => mount(<MyComponent />)).toThrowErrorMatchingSnapshot()
 })
 
-test('not applying the onClick prop results in an error', () => {
-  const MyComponent = () => (
-    <Downshift>
-      {({getRootProps}) => {
-        const {ref} = getRootProps()
-        return <div ref={ref} />
-      }}
-    </Downshift>
-  )
-  expect(() => mount(<MyComponent />)).toThrowErrorMatchingSnapshot()
-})
-
 test('renders fine when rendering a composite component and applying getRootProps properly', () => {
   const MyComponent = () => (
     <Downshift>

src/downshift.js

@@ -5,13 +5,11 @@ import PropTypes from 'prop-types'
 import setA11yStatus from './set-a11y-status'
 import {
   cbToCb,
-  findParent,
   composeEventHandlers,
   debounce,
   scrollIntoView,
   generateId,
   firstDefined,
-  isNumber,
   getA11yStatusMessage,
   unwrapArray,
   isDOMElement,
@@ -370,35 +368,17 @@ class Downshift extends Component {
 
   rootRef = node => (this._rootNode = node)
 
-  getRootProps = ({refKey = 'ref', onClick, ...rest} = {}) => {
+  getRootProps = ({refKey = 'ref', ...rest} = {}) => {
     // this is used in the render to know whether the user has called getRootProps.
     // It uses that to know whether to apply the props automatically
     this.getRootProps.called = true
     this.getRootProps.refKey = refKey
     return {
       [refKey]: this.rootRef,
-      onClick: composeEventHandlers(onClick, this.root_handleClick),
       ...rest,
     }
   }
 
-  root_handleClick = event => {
-    event.preventDefault()
-    const itemParent = findParent(
-      node => {
-        const index = this.getItemIndexFromId(node.getAttribute('id'))
-        return isNumber(index)
-      },
-      event.target,
-      this._rootNode,
-    )
-    if (itemParent) {
-      this.selectItemAtIndex(
-        this.getItemIndexFromId(itemParent.getAttribute('id')),
-      )
-    }
-  }
-
   //\\\\\\\\\\\\\\\\\\\\\\\\\\ ROOT
 
   keyDownHandlers = {
@@ -566,17 +546,10 @@ class Downshift extends Component {
     return `${this.id}-item-${index}`
   }
 
-  getItemIndexFromId(id) {
-    if (id) {
-      return Number(id.split(`${this.id}-item-`)[1])
-    } else {
-      return null
-    }
-  }
-
   getItemProps = (
     {
       onMouseEnter,
+      onClick,
       index,
       item = requiredProp('getItemProps', 'item'),
       ...rest
@@ -595,6 +568,9 @@ class Downshift extends Component {
           type: Downshift.stateChangeTypes.itemMouseEnter,
         })
       }),
+      onClick: composeEventHandlers(onClick, () => {
+        this.selectItemAtIndex(index)
+      }),
       ...rest,
     }
   }
@@ -766,9 +742,4 @@ function validateGetRootPropsCalledCorrectly(element, {refKey}) {
       `downshift: You must apply the ref prop "${refKey}" from getRootProps onto your root element.`,
     )
   }
-  if (!getElementProps(element).hasOwnProperty('onClick')) {
-    throw new Error(
-      `downshift: You must apply the "onClick" prop from getRootProps onto your root element.`,
-    )
-  }
 }

src/utils.js

@@ -146,12 +146,6 @@ function firstDefined(...args) {
   return args.find(a => typeof a !== 'undefined')
 }
 
-function isNumber(thing) {
-  // not NaN and is a number type
-  // eslint-disable-next-line no-self-compare
-  return thing === thing && typeof thing === 'number'
-}
-
 // eslint-disable-next-line complexity
 function getA11yStatusMessage({
   isOpen,
@@ -242,7 +236,7 @@ const stateKeys = [
  */
 function pickState(state = {}) {
   const result = {}
-  stateKeys.forEach((k) => {
+  stateKeys.forEach(k => {
     if (state.hasOwnProperty(k)) {
       result[k] = state[k]
     }
@@ -252,13 +246,11 @@ function pickState(state = {}) {
 
 export {
   cbToCb,
-  findParent,
   composeEventHandlers,
   debounce,
   scrollIntoView,
   generateId,
   firstDefined,
-  isNumber,
   getA11yStatusMessage,
   unwrapArray,
   isDOMElement,