feat(common): Improve shorthand array handling

This updates the createShorthand factory to take two special props as
extraProps to generate a key if missing. This is necessary for shorthand
props that are arrays. The props are:
- childKey - enables explicitly setting the key via props
- getChildKey - function that takes the given props and returns a key

This also fixes some issues I noticed using the menu `items` shorthand prop:
- Use the createShorthand factory to create a MenuItem for each item.
- Do not infer activeIndex from the items array on mount.

Regarding the second point, there were a few issues with it:
- The array items may not be objects - they could be primitives or elements
- You should be able to reliably set the activeIndex or defaultActiveIndex
via props, but this would overwrite that.

src/collections/Menu/Menu.js

@@ -14,6 +14,7 @@ import {
   useValueAndKey,
   useWidthProp,
 } from '../../lib'
+import { createShorthand } from '../../factories'
 import MenuHeader from './MenuHeader'
 import MenuItem from './MenuItem'
 import MenuMenu from './MenuMenu'
@@ -92,14 +93,12 @@ class Menu extends Component {
     /** Shorthand array of props for Menu. Mutually exclusive with children. */
     items: customPropTypes.every([
       customPropTypes.disallow(['children']),
-      PropTypes.arrayOf(PropTypes.shape({
-        childKey: PropTypes.oneOfType([
-          PropTypes.number,
-          PropTypes.string,
-        ]),
-        // this object is spread on the MenuItem
-        // allow it to validate props instead
-      })),
+      // Array of shorthands for MenuItem
+      PropTypes.arrayOf(PropTypes.oneOfType([
+        PropTypes.string,
+        PropTypes.element,
+        PropTypes.object,
+      ])),
     ]),
 
     /** onClick handler for MenuItem. Mutually exclusive with children. */
@@ -149,13 +148,6 @@ class Menu extends Component {
   static Item = MenuItem
   static Menu = MenuMenu
 
-  componentWillMount() {
-    super.componentWillMount()
-
-    const { items } = this.props
-    if (items) this.trySetState({ activeIndex: _.findIndex(items, ['active', true]) })
-  }
-
   handleItemClick = (e, { name, index }) => {
     this.trySetState({ activeIndex: index })
     const { items, onItemClick } = this.props
@@ -169,20 +161,12 @@ class Menu extends Component {
     const { activeIndex } = this.state
 
     return _.map(items, (item, index) => {
-      const { content, childKey, name, itemProps } = item
-      const finalKey = childKey || [content, name].join('-')
-
-      return (
-        <MenuItem
-          {...itemProps}
-          active={activeIndex === index}
-          content={content}
-          index={index}
-          key={finalKey}
-          name={name}
-          onClick={this.handleItemClick}
-        />
-      )
+      return createShorthand(MenuItem, val => ({ content: val }), item, {
+        active: activeIndex === index,
+        getChildKey: ({ content, name }) => [content, name].join('-'),
+        index,
+        onClick: this.handleItemClick,
+      })
     })
   }
 

src/factories.js

@@ -6,6 +6,22 @@ import Icon from './elements/Icon/Icon'
 import Image from './elements/Image/Image'
 import Label from './elements/Label/Label'
 
+/**
+ * If the 'key' prop is not set, set it based on the childKey or getChildKey props.
+ *
+ * @param   {function} options.getChildKey Returns a key based on props
+ * @param   {string} options.childKey Given childKey
+ * @param   {object} options.props A props object
+ * @returns {object} A new props object
+ */
+const mergeChildKey = ({ getChildKey, childKey, ...props }) => {
+  if (!props.key) {
+    props.key = childKey || getChildKey && getChildKey(props)
+  }
+
+  return props
+}
+
 /**
  * Merges props and classNames.
  *
@@ -20,7 +36,7 @@ const mergePropsAndClassName = (props, extraProps) => {
     newProps.className = cx(props.className, extraProps.className) // eslint-disable-line react/prop-types
   }
 
-  return newProps
+  return mergeChildKey(newProps)
 }
 
 /**

test/specs/collections/Menu/Menu-test.js

@@ -1,3 +1,4 @@
+import _ from 'lodash'
 import React from 'react'
 
 import Menu from 'src/collections/Menu/Menu'
@@ -39,30 +40,33 @@ describe('Menu', () => {
   })
 
   describe('activeIndex', () => {
-    it('-1 by default', () => {
-      const items = [
-        { name: 'home' },
-        { name: 'users' },
-      ]
+    const items = [
+      { name: 'home' },
+      { name: 'users' },
+    ]
 
-      shallow(<Menu items={items} />).should.have.state('activeIndex', -1)
+    it('is null by default', () => {
+      shallow(<Menu items={items} />)
+        .should.not.have.descendants('.active')
     })
 
-    it('can be overridden with "active"', () => {
-      const items = [
-        { name: 'home' },
-        { active: true, name: 'users' },
-      ]
+    it('is set when clicking an item', () => {
+      // random item, skip the first as its selected by default
+      const randomIndex = _.random(items.length - 1)
 
-      shallow(<Menu items={items} />).should.have.state('activeIndex', 1)
+      mount(<Menu items={items} />)
+        .find('MenuItem')
+        .at(randomIndex)
+        .simulate('click')
+        .should.have.prop('active', true)
     })
   })
 
   describe('items', () => {
     const spy = sandbox.spy()
     const items = [
-      { name: 'home', onClick: spy },
-      { active: true, name: 'users' },
+      { name: 'home', onClick: spy, 'data-foo': 'something' },
+      { name: 'users', active: true, 'data-foo': 'something' },
     ]
     const children = mount(<Menu items={items} />).find('MenuItem')
 
@@ -85,6 +89,10 @@ describe('Menu', () => {
       spy.should.have.been.calledOnce()
       spy.should.have.been.calledWithMatch(event, props)
     })
+
+    it('passes arbitraty props', () => {
+      children.everyWhere(item => item.should.have.prop('data-foo', 'something'))
+    })
   })
 
   describe('onItemClick', () => {