From 32d5d69ff28f922527fc1a7f51563961abba9d33 Mon Sep 17 00:00:00 2001
From: Samuel Galson <samgalson@gmail.com>
Date: Wed, 31 Jan 2018 17:30:34 +0000
Subject: [PATCH] feat(ui): remove name prop and refactor

BREAKING CHANGE: StateItem doesn't accept the 'name' prop anymore, as it wasn't used.
---
 packages/ui/src/atoms/StateItem.js            | 12 +++----
 packages/ui/src/atoms/StateItem.md            | 32 +++++++++++++------
 packages/ui/src/molecules/StateList.js        | 13 ++++----
 packages/ui/test/StateItem.test.js            | 15 ++++-----
 packages/ui/test/StateList.test.js            |  2 +-
 .../test/__snapshots__/StateItem.test.js.snap | 20 +++++++++++-
 .../test/__snapshots__/StateList.test.js.snap | 26 ++++++++++++---
 7 files changed, 82 insertions(+), 38 deletions(-)

diff --git a/packages/ui/src/atoms/StateItem.js b/packages/ui/src/atoms/StateItem.js
index 3931c08c8..870a639d9 100644
--- a/packages/ui/src/atoms/StateItem.js
+++ b/packages/ui/src/atoms/StateItem.js
@@ -33,12 +33,11 @@ const Root = styled.span`
   ${props => (props.disabled ? disabled : '')};
 `
 
-const StateItem = ({ disabled, name, update, values, index }) => {
-  const handleInteraction = () => {
+const StateItem = ({ update, disabled, values, index }) => {
+  const callUpdateWithNextIndex = () => {
     if (disabled) return
-
     const nextIndex = arrayShift(values, index)
-    update(name, nextIndex)
+    update(values[index], nextIndex)
   }
 
   const arrayShift = (array, i) => (i === array.length - 1 ? 0 : i + 1)
@@ -46,8 +45,8 @@ const StateItem = ({ disabled, name, update, values, index }) => {
   return (
     <Root
       disabled={disabled}
-      onClick={handleInteraction}
-      onKeyPress={handleInteraction}
+      onClick={callUpdateWithNextIndex}
+      onKeyPress={callUpdateWithNextIndex}
       role="button"
       tabIndex="0"
     >
@@ -59,7 +58,6 @@ const StateItem = ({ disabled, name, update, values, index }) => {
 StateItem.propTypes = {
   disabled: PropTypes.bool,
   index: PropTypes.number.isRequired,
-  name: PropTypes.string.isRequired,
   update: PropTypes.func.isRequired,
   values: PropTypes.arrayOf(PropTypes.string).isRequired,
 }
diff --git a/packages/ui/src/atoms/StateItem.md b/packages/ui/src/atoms/StateItem.md
index 73b351ddb..8fda4f6ec 100644
--- a/packages/ui/src/atoms/StateItem.md
+++ b/packages/ui/src/atoms/StateItem.md
@@ -1,22 +1,34 @@
-An interactive element which upon click changes its text content. The available text values should be provided as an array of strings. The actual operation which takes place is a right shift on the array of provided values
+An interactive element which renders a text element from a given index of a provided array and upon click runs a provided update function, passing it the current text element and the index of the next item in the array, or 0 if the current item is the last.
+
+If the update function is used to update the index, the component will cycle through the array on click.
 
 ```js
-const data = {
+const initialState = {
   values: ['To Clean', 'Cleaning', 'Cleaned'],
   index: 0,
-  disabled: false,
-  name: 'clean',
 }
 
-const update = value => {
-  console.log('value', value)
+const update = (currentValue, nextIndex) => {
+  setState({ index: nextIndex })
 }
+;<StateItem values={state.values} index={state.index} update={update} />
+```
+
+If the component is passed the disabled prop, the update function is not run:
 
+```js
+const initialState = {
+  values: ['To Clean', 'Cleaning', 'Cleaned'],
+  index: 0,
+}
+
+const update = (currentValue, nextIndex) => {
+  setState({ index: nextIndex })
+}
 ;<StateItem
-  values={data.values}
-  disabled={data.disabled}
+  values={state.values}
+  index={state.index}
+  disabled={true}
   update={update}
-  index={data.index}
-  name={data.name}
 />
 ```
diff --git a/packages/ui/src/molecules/StateList.js b/packages/ui/src/molecules/StateList.js
index a9c964c86..9345a486b 100644
--- a/packages/ui/src/molecules/StateList.js
+++ b/packages/ui/src/molecules/StateList.js
@@ -13,24 +13,23 @@ const StateList = ({ currentValues, update, values }) => {
   // TODO: Placeholder -- to be implemented with authsome
   const canAct = key => true
 
-  const handleUpdate = (name, index) => {
-    update(name, index)
+  const handleUpdate = (currentItem, nextIndex) => {
+    update(currentItem, nextIndex)
   }
 
-  const items = map(values, (valueList, name) => {
+  const items = map(values, (valueList, currentItem) => {
     let delimiter
-    const currentValueIndex = currentValues[name]
+    const currentValueIndex = currentValues[currentItem]
 
-    if (name !== lastItem) {
+    if (currentItem !== lastItem) {
       delimiter = <ChevronRight className={classes.delimiter} size={16} />
     }
 
     return (
       <div className={classes.itemContainer} key={uniqueId()}>
         <StateItem
-          disabled={!canAct(name)}
+          disabled={!canAct(currentItem)}
           index={currentValueIndex}
-          name={name}
           update={handleUpdate}
           values={valueList}
         />
diff --git a/packages/ui/test/StateItem.test.js b/packages/ui/test/StateItem.test.js
index 3fb631c16..86faa3f85 100644
--- a/packages/ui/test/StateItem.test.js
+++ b/packages/ui/test/StateItem.test.js
@@ -1,6 +1,7 @@
 import React from 'react'
 import { clone } from 'lodash'
 import { shallow, render } from 'enzyme'
+import 'jest-styled-components'
 import renderer from 'react-test-renderer'
 
 import StateItem from '../src/atoms/StateItem'
@@ -8,10 +9,8 @@ import StateItem from '../src/atoms/StateItem'
 const myMock = jest.fn()
 const props = {
   values: ['To Clean', 'Cleaning', 'Cleaned'],
-  disabled: false,
   update: myMock,
   index: 1,
-  name: 'clean',
 }
 
 const wrapper = shallow(<StateItem {...props} />)
@@ -23,16 +22,16 @@ describe('StateItem', () => {
     expect(tree).toMatchSnapshot()
   })
 
-  test('with default props class disabled should not exist', () => {
-    expect(wrapper.is('.disabled')).toBe(false)
+  test('with default props cursor should be pointer', () => {
+    const tree = renderer.create(<StateItem {...props} />).toJSON()
+    expect(tree).toHaveStyleRule('cursor', 'pointer')
   })
 
-  test('with given props should be disabled', () => {
+  test('with disabled prop cursor should be default', () => {
     const newProps = clone(props)
     newProps.disabled = true
-    const newWrapper = shallow(<StateItem {...newProps} />)
-
-    expect(newWrapper.is('.disabled')).toBe(true)
+    const tree = renderer.create(<StateItem {...newProps} />).toJSON()
+    expect(tree).toHaveStyleRule('cursor', 'default')
   })
 
   test('should render the value Cleaning', () => {
diff --git a/packages/ui/test/StateList.test.js b/packages/ui/test/StateList.test.js
index aa56cfe13..750b0871c 100644
--- a/packages/ui/test/StateList.test.js
+++ b/packages/ui/test/StateList.test.js
@@ -1,6 +1,7 @@
 import React from 'react'
 import { forIn } from 'lodash'
 import { shallow } from 'enzyme'
+import 'jest-styled-components'
 import renderer from 'react-test-renderer'
 
 import StateItem from '../src/atoms/StateItem'
@@ -50,7 +51,6 @@ describe('StateList', () => {
 
       expect(stateItemProps.disabled).toEqual(false)
       expect(stateItemProps.index).toEqual(props.currentValues[key])
-      expect(stateItemProps.name).toEqual(key)
       expect(stateItemProps.values).toEqual(props.values[key])
 
       i += 1
diff --git a/packages/ui/test/__snapshots__/StateItem.test.js.snap b/packages/ui/test/__snapshots__/StateItem.test.js.snap
index 0ed454eec..3f77c3a5f 100644
--- a/packages/ui/test/__snapshots__/StateItem.test.js.snap
+++ b/packages/ui/test/__snapshots__/StateItem.test.js.snap
@@ -1,8 +1,26 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`StateItem is rendered correctly 1`] = `
+.c0 {
+  cursor: pointer;
+  font-family: var(--font-interface);
+  font-size: 16px;
+  font-style: italic;
+  padding: 0;
+}
+
+.c0:focus {
+  outline: none;
+}
+
+.c0:hover {
+  color: #404040;
+  -webkit-transition: 0.25s ease-in-out 0s;
+  transition: 0.25s ease-in-out 0s;
+}
+
 <span
-  className="root"
+  className="c0"
   disabled={false}
   onClick={[Function]}
   onKeyPress={[Function]}
diff --git a/packages/ui/test/__snapshots__/StateList.test.js.snap b/packages/ui/test/__snapshots__/StateList.test.js.snap
index 4a07e16e4..9d2b62183 100644
--- a/packages/ui/test/__snapshots__/StateList.test.js.snap
+++ b/packages/ui/test/__snapshots__/StateList.test.js.snap
@@ -1,6 +1,24 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`StateList is rendered correctly 1`] = `
+.c0 {
+  cursor: pointer;
+  font-family: var(--font-interface);
+  font-size: 16px;
+  font-style: italic;
+  padding: 0;
+}
+
+.c0:focus {
+  outline: none;
+}
+
+.c0:hover {
+  color: #404040;
+  -webkit-transition: 0.25s ease-in-out 0s;
+  transition: 0.25s ease-in-out 0s;
+}
+
 <div
   className="stateListContainer"
 >
@@ -8,7 +26,7 @@ exports[`StateList is rendered correctly 1`] = `
     className="itemContainer"
   >
     <span
-      className="root"
+      className="c0"
       disabled={false}
       onClick={[Function]}
       onKeyPress={[Function]}
@@ -38,7 +56,7 @@ exports[`StateList is rendered correctly 1`] = `
     className="itemContainer"
   >
     <span
-      className="root"
+      className="c0"
       disabled={false}
       onClick={[Function]}
       onKeyPress={[Function]}
@@ -68,7 +86,7 @@ exports[`StateList is rendered correctly 1`] = `
     className="itemContainer"
   >
     <span
-      className="root"
+      className="c0"
       disabled={false}
       onClick={[Function]}
       onKeyPress={[Function]}
@@ -98,7 +116,7 @@ exports[`StateList is rendered correctly 1`] = `
     className="itemContainer"
   >
     <span
-      className="root"
+      className="c0"
       disabled={false}
       onClick={[Function]}
       onKeyPress={[Function]}
-- 
GitLab