From a159b9aef245bb79ff4ae39e03b9e8a59dd2a5de Mon Sep 17 00:00:00 2001
From: Laura Cabrera <lzcabrera@gmail.com>
Date: Thu, 14 Sep 2017 13:03:00 -0700
Subject: [PATCH] refactor(ordered-list): Code clean up

---
 .../Lists/OrderedList/OrderedItem.jsx         | 15 +++------
 .../Lists/OrderedList/OrderedList.jsx         | 31 ++++++------------
 .../__tests__/OrderedList.spec.jsx            | 32 ++++++++-----------
 .../__snapshots__/OrderedList.spec.jsx.snap   |  4 +--
 4 files changed, 29 insertions(+), 53 deletions(-)

diff --git a/src/components/Lists/OrderedList/OrderedItem.jsx b/src/components/Lists/OrderedList/OrderedItem.jsx
index 58ab5995ab..1067cc25b3 100644
--- a/src/components/Lists/OrderedList/OrderedItem.jsx
+++ b/src/components/Lists/OrderedList/OrderedItem.jsx
@@ -5,22 +5,17 @@ import safeRest from '../../../safeRest'
 
 import styles from '../../Typography/Text/Text.modules.scss'
 
-const OrderedItem = ({ size, children, ...rest }) => {
-  return (
-    <li {...safeRest(rest)} className={styles[size]}>
-      {children}
-    </li>
-  )
-}
+const OrderedItem = ({ size, children, ...rest }) => (
+  <li {...safeRest(rest)} className={styles[size]}>
+    {children}
+  </li>
+)
 
 OrderedItem.propTypes = {
   size: PropTypes.oneOf([
     'medium',
     'large'
   ]),
-  /**
-   * The content
-   */
   children: PropTypes.node.isRequired
 }
 
diff --git a/src/components/Lists/OrderedList/OrderedList.jsx b/src/components/Lists/OrderedList/OrderedList.jsx
index b8df16f417..690c29baed 100644
--- a/src/components/Lists/OrderedList/OrderedList.jsx
+++ b/src/components/Lists/OrderedList/OrderedList.jsx
@@ -7,28 +7,17 @@ import OrderedItem from './OrderedItem'
 
 import styles from './OrderedList.modules.scss'
 
-const OrderedList = ({ listStyle, size, children, ...rest }) => {
-  const classes = `
-    ${styles[listStyle]}
-  `
-
-  const sizeChildren = child =>
-    React.cloneElement(child, {
-      size
-    })
-
-  const items = React.Children.map(children, sizeChildren)
-
-  return (
-    <ol {...safeRest(rest)} className={classes}>
-      {items}
-    </ol>
-  )
-}
+const injectSize = (child, size) => React.cloneElement(child, { size })
+
+const OrderedList = ({ listStyle, size, children, ...rest }) => (
+  <ol {...safeRest(rest)} className={styles[listStyle]}>
+    {React.Children.map(children, child => injectSize(child, size))}
+  </ol>
+)
 
 OrderedList.propTypes = {
   /**
-   * The type of list.
+   * The bullet style.
    */
   listStyle: PropTypes.oneOf([
     'decimal',
@@ -36,14 +25,14 @@ OrderedList.propTypes = {
     'lowerAlpha'
   ]),
   /**
-   * The font size
+   * The font size.
    */
   size: PropTypes.oneOf([
     'medium',
     'large'
   ]),
   /**
-   * The list items.
+   * The list items. Must be at least one `OrderedList.Item`.
    */
   children: childrenOfType(OrderedItem).isRequired
 }
diff --git a/src/components/Lists/OrderedList/__tests__/OrderedList.spec.jsx b/src/components/Lists/OrderedList/__tests__/OrderedList.spec.jsx
index f7cc924ebb..d2d133b839 100644
--- a/src/components/Lists/OrderedList/__tests__/OrderedList.spec.jsx
+++ b/src/components/Lists/OrderedList/__tests__/OrderedList.spec.jsx
@@ -12,19 +12,13 @@ describe('<OrderedList />', () => {
     </OrderedList>
   )
 
-  const doShallowList = (overrides = {}) => shallow(
+  const doShallow = (overrides = {}) => shallow(
     <OrderedList {...overrides}>
       <OrderedList.Item>Lorem ipsum</OrderedList.Item>
       <OrderedList.Item>Dolor sit amet</OrderedList.Item>
     </OrderedList>
   )
 
-  const findOrderedList = orderedList => orderedList.find('ol')
-
-  const doShallowItem = (overrides = {}) => shallow(
-    <OrderedList.Item {...overrides}>Some content</OrderedList.Item>
-  )
-
   it('renders', () => {
     const orderedList = doRender()
 
@@ -32,36 +26,36 @@ describe('<OrderedList />', () => {
   })
 
   it('OrderList renders an HTML ol tag', () => {
-    const orderedList = doShallowList()
+    const orderedList = doShallow()
 
     expect(orderedList).toHaveTagName('ol')
   })
 
   it('OrderList.Item renders an HTML li tag', () => {
-    const orderedListItem = doShallowItem()
+    const orderedListItem = shallow(<OrderedList.Item>a list item</OrderedList.Item>)
 
     expect(orderedListItem).toHaveTagName('li')
   })
 
   it('can have a list style', () => {
-    let orderedList = doShallowList({ listStyle: undefined })
-    expect(findOrderedList(orderedList)).toHaveClassName('decimal')
+    let orderedList = doShallow({ listStyle: undefined })
+    expect(orderedList).toHaveClassName('decimal')
 
-    orderedList = doShallowList({ listStyle: 'upperAlpha' })
-    expect(findOrderedList(orderedList)).toHaveClassName('upperAlpha')
+    orderedList = doShallow({ listStyle: 'upperAlpha' })
+    expect(orderedList).toHaveClassName('upperAlpha')
   })
 
   it('passes additional attributes to ol element', () => {
-    const orderedList = doShallowList({ id: 'the-list', tabindex: 1 })
+    const orderedList = doShallow({ id: 'the-list', reversed: 'true' })
 
-    expect(findOrderedList(orderedList)).toHaveProp('id', 'the-list')
-    expect(findOrderedList(orderedList)).toHaveProp('tabindex', 1)
+    expect(orderedList).toHaveProp('id', 'the-list')
+    expect(orderedList).toHaveProp('reversed', 'true')
   })
 
   it('does not allow custom CSS', () => {
-    const orderedList = doShallowList({ className: 'my-custom-class', style: { color: 'hotpink' } })
+    const orderedList = doShallow({ className: 'my-custom-class', style: { color: 'hotpink' } })
 
-    expect(findOrderedList(orderedList)).not.toHaveProp('className', 'my-custom-class')
-    expect(findOrderedList(orderedList)).not.toHaveProp('style')
+    expect(orderedList).not.toHaveProp('className', 'my-custom-class')
+    expect(orderedList).not.toHaveProp('style')
   })
 })
diff --git a/src/components/Lists/OrderedList/__tests__/__snapshots__/OrderedList.spec.jsx.snap b/src/components/Lists/OrderedList/__tests__/__snapshots__/OrderedList.spec.jsx.snap
index e75b6ab349..5da5b77092 100644
--- a/src/components/Lists/OrderedList/__tests__/__snapshots__/OrderedList.spec.jsx.snap
+++ b/src/components/Lists/OrderedList/__tests__/__snapshots__/OrderedList.spec.jsx.snap
@@ -2,9 +2,7 @@
 
 exports[`<OrderedList /> renders 1`] = `
 <ol
-  class="
-    decimal
-  "
+  class="decimal"
 >
   <li
     class="undefined"