From 774c5586b7061902edc58afd62430d6d9be7064d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafa=C5=82=20Kostrzewski?= <poczta.raf.kos@gmail.com>
Date: Wed, 22 Feb 2017 01:28:22 +0100
Subject: [PATCH] feat(Accordion): support panel's title as custom element
 #1144 (#1281)

* feat(Accordion): support panel's title as custom element #1144

* refactor(Accordion) panels created using createShorthandFactory

* fix(Accordion) title and content custom elements not wrapped in div with class names

* docs(Accordion) add example use case for custom title component to Accordion docs

* refactor(Accordion): remove unused import

* Revert "docs(Accordion) add example use case for custom title component to Accordion docs"

This reverts commit 1b84e22af48354c14c7099615b09da1c3aee02b5.

* fix(Accordion): Accordion Title does not have dropdown for panels with string title

* docs(Accordion): added separate example for Accordion with custom title and content

* feat(Accordion): Accordion consistently renders dropdown icon for panels' titles

* docs(Accordion): Move custom title and content example to Usages

* docs(Accordion): reorganize panels shorthand

* feat(Accordion): support all child keys in panels
---
 .../Examples/modules/Accordion/Types/index.js | 14 +---
 .../AccordionExampleActiveIndex.js            |  0
 ...mplePanelsPropWithCustomTitleAndContent.js | 22 ++++++
 .../Examples/modules/Accordion/Usage/index.js | 28 ++++++++
 docs/app/Examples/modules/Accordion/index.js  |  2 +
 src/modules/Accordion/Accordion.js            | 27 ++++----
 src/modules/Accordion/AccordionContent.js     | 17 ++++-
 src/modules/Accordion/AccordionTitle.js       | 25 ++++++-
 src/modules/Accordion/index.d.ts              |  3 +-
 .../specs/modules/Accordion/Accordion-test.js | 68 ++++++++++++++++---
 .../Accordion/AccordionContent-test.js        |  1 +
 .../modules/Accordion/AccordionTitle-test.js  |  1 +
 12 files changed, 165 insertions(+), 43 deletions(-)
 rename docs/app/Examples/modules/Accordion/{Types => Usage}/AccordionExampleActiveIndex.js (100%)
 create mode 100644 docs/app/Examples/modules/Accordion/Usage/AccordionExamplePanelsPropWithCustomTitleAndContent.js
 create mode 100644 docs/app/Examples/modules/Accordion/Usage/index.js

diff --git a/docs/app/Examples/modules/Accordion/Types/index.js b/docs/app/Examples/modules/Accordion/Types/index.js
index 27e4cd1550..c1db7b86ab 100644
--- a/docs/app/Examples/modules/Accordion/Types/index.js
+++ b/docs/app/Examples/modules/Accordion/Types/index.js
@@ -12,8 +12,7 @@ const AccordionTypesExamples = () => (
       examplePath='modules/Accordion/Types/AccordionExampleStandard'
     />
     <ComponentExample
-      title='Panels Prop'
-      description='Accordion panels can be define using the `panels` prop.'
+      description={<div>An Accordion can be defined using the <code>panels</code> prop.</div>}
       examplePath='modules/Accordion/Types/AccordionExamplePanelsProp'
     >
       <Message info>
@@ -21,17 +20,6 @@ const AccordionTypesExamples = () => (
         {' '}They can also define an <code>onClick</code> key to be applied to the <code>Accordion.Title</code>.
       </Message>
     </ComponentExample>
-    <ComponentExample
-      title='Active Index'
-      description='The `activeIndex` prop controls which panel is open.'
-      examplePath='modules/Accordion/Types/AccordionExampleActiveIndex'
-    >
-      <Message info>
-        An <code>active</code> prop on an
-        {' '}<code>&lt;Accordion.Title&gt;</code> or <code>&lt;Accordion.Content&gt;</code>
-        {' '}will override the <code>&lt;Accordion&gt;</code> <code>&lt;activeIndex&gt;</code> prop.
-      </Message>
-    </ComponentExample>
     <ComponentExample
       title='Styled'
       description='A styled accordion adds basic formatting.'
diff --git a/docs/app/Examples/modules/Accordion/Types/AccordionExampleActiveIndex.js b/docs/app/Examples/modules/Accordion/Usage/AccordionExampleActiveIndex.js
similarity index 100%
rename from docs/app/Examples/modules/Accordion/Types/AccordionExampleActiveIndex.js
rename to docs/app/Examples/modules/Accordion/Usage/AccordionExampleActiveIndex.js
diff --git a/docs/app/Examples/modules/Accordion/Usage/AccordionExamplePanelsPropWithCustomTitleAndContent.js b/docs/app/Examples/modules/Accordion/Usage/AccordionExamplePanelsPropWithCustomTitleAndContent.js
new file mode 100644
index 0000000000..5905dbb705
--- /dev/null
+++ b/docs/app/Examples/modules/Accordion/Usage/AccordionExamplePanelsPropWithCustomTitleAndContent.js
@@ -0,0 +1,22 @@
+import React from 'react'
+import { Accordion, Label, Message } from 'semantic-ui-react'
+import faker from 'faker'
+import _ from 'lodash'
+
+const panels = _.times(3, i => ({
+  key: `panel-${i}`,
+  title: <Label color='blue' content={faker.lorem.sentence()} />,
+  content: (
+    <Message
+      info
+      header={faker.lorem.sentence()}
+      content={faker.lorem.paragraph()}
+    />
+  ),
+}))
+
+const AccordionExamplePanelsPropWithCustomTitleAndContent = () => (
+  <Accordion panels={panels} />
+)
+
+export default AccordionExamplePanelsPropWithCustomTitleAndContent
diff --git a/docs/app/Examples/modules/Accordion/Usage/index.js b/docs/app/Examples/modules/Accordion/Usage/index.js
new file mode 100644
index 0000000000..d937e4317d
--- /dev/null
+++ b/docs/app/Examples/modules/Accordion/Usage/index.js
@@ -0,0 +1,28 @@
+import React from 'react'
+
+import ComponentExample from 'docs/app/Components/ComponentDoc/ComponentExample'
+import ExampleSection from 'docs/app/Components/ComponentDoc/ExampleSection'
+
+import { Message } from 'semantic-ui-react'
+
+const AccordionUsageExamples = () => (
+  <ExampleSection title='Usage'>
+    <ComponentExample
+      title='Active Index'
+      description='The `activeIndex` prop controls which panel is open.'
+      examplePath='modules/Accordion/Usage/AccordionExampleActiveIndex'
+    >
+      <Message info>
+        An <code>active</code> prop on an
+        {' '}<code>&lt;Accordion.Title&gt;</code> or <code>&lt;Accordion.Content&gt;</code>
+        {' '}will override the <code>&lt;Accordion&gt;</code> <code>&lt;activeIndex&gt;</code> prop.
+      </Message>
+    </ComponentExample>
+    <ComponentExample
+      title='Panels Prop with custom title and content'
+      examplePath='modules/Accordion/Usage/AccordionExamplePanelsPropWithCustomTitleAndContent'
+    />
+  </ExampleSection>
+)
+
+export default AccordionUsageExamples
diff --git a/docs/app/Examples/modules/Accordion/index.js b/docs/app/Examples/modules/Accordion/index.js
index a452332fa4..5b5da95be1 100644
--- a/docs/app/Examples/modules/Accordion/index.js
+++ b/docs/app/Examples/modules/Accordion/index.js
@@ -1,11 +1,13 @@
 import React from 'react'
 import Types from './Types'
 import Variations from './Variations'
+import Usage from './Usage'
 
 const AccordionExamples = () => (
   <div>
     <Types />
     <Variations />
+    <Usage />
   </div>
 )
 
diff --git a/src/modules/Accordion/Accordion.js b/src/modules/Accordion/Accordion.js
index a11d541c6c..42523aed49 100644
--- a/src/modules/Accordion/Accordion.js
+++ b/src/modules/Accordion/Accordion.js
@@ -1,5 +1,5 @@
-import cx from 'classnames'
 import _ from 'lodash'
+import cx from 'classnames'
 import React, { Children, cloneElement, PropTypes } from 'react'
 
 import {
@@ -9,7 +9,7 @@ import {
   META,
   useKeyOnly,
 } from '../../lib'
-import Icon from '../../elements/Icon'
+
 import AccordionContent from './AccordionContent'
 import AccordionTitle from './AccordionTitle'
 
@@ -54,14 +54,16 @@ export default class Accordion extends Component {
     /**
      * Create simple accordion panels from an array of { text: <string>, content: <custom> } objects.
      * Object can optionally define an `active` key to open/close the panel.
+     * Object can opitonally define a `key` key used for title and content nodes' keys.
      * Mutually exclusive with children.
      * TODO: AccordionPanel should be a sub-component
      */
     panels: customPropTypes.every([
       customPropTypes.disallow(['children']),
       PropTypes.arrayOf(PropTypes.shape({
+        key: PropTypes.string,
         active: PropTypes.bool,
-        title: PropTypes.string,
+        title: customPropTypes.contentShorthand,
         content: customPropTypes.contentShorthand,
         onClick: PropTypes.func,
       })),
@@ -158,17 +160,14 @@ export default class Accordion extends Component {
         if (panel.onClick) panel.onClick(e, i)
       }
 
-      children.push(
-        <AccordionTitle key={`${panel.title}-title`} active={isActive} onClick={onClick}>
-          <Icon name='dropdown' />
-          {panel.title}
-        </AccordionTitle>
-      )
-      children.push(
-        <AccordionContent key={`${panel.title}-content`} active={isActive}>
-          {panel.content}
-        </AccordionContent>
-      )
+      // implement all methods of creating a key that are supported in factories
+      const key = panel.key
+        || _.isFunction(panel.childKey) && panel.childKey(panel)
+        || panel.childKey && panel.childKey
+        || panel.title
+
+      children.push(AccordionTitle.create({ active: isActive, onClick, key: `${key}-title`, content: panel.title }))
+      children.push(AccordionContent.create({ active: isActive, key: `${key}-content`, content: panel.content }))
     })
 
     return children
diff --git a/src/modules/Accordion/AccordionContent.js b/src/modules/Accordion/AccordionContent.js
index 2a5e509953..1865cc5692 100644
--- a/src/modules/Accordion/AccordionContent.js
+++ b/src/modules/Accordion/AccordionContent.js
@@ -1,5 +1,6 @@
-import cx from 'classnames'
+import _ from 'lodash'
 import React, { PropTypes } from 'react'
+import cx from 'classnames'
 
 import {
   customPropTypes,
@@ -7,13 +8,14 @@ import {
   getUnhandledProps,
   META,
   useKeyOnly,
+  createShorthandFactory,
 } from '../../lib'
 
 /**
  * A content sub-component for Accordion component.
  */
 function AccordionContent(props) {
-  const { active, children, className } = props
+  const { active, children, className, content } = props
   const classes = cx(
     'content',
     useKeyOnly(active, 'active'),
@@ -22,7 +24,11 @@ function AccordionContent(props) {
   const rest = getUnhandledProps(AccordionContent, props)
   const ElementType = getElementType(AccordionContent, props)
 
-  return <ElementType {...rest} className={classes}>{children}</ElementType>
+  return (
+    <ElementType {...rest} className={classes}>
+      {_.isNil(children) ? content : children}
+    </ElementType>
+  )
 }
 
 AccordionContent.propTypes = {
@@ -37,6 +43,9 @@ AccordionContent.propTypes = {
 
   /** Additional classes. */
   className: PropTypes.string,
+
+  /** Shorthand for primary content. */
+  content: customPropTypes.contentShorthand,
 }
 
 AccordionContent._meta = {
@@ -45,4 +54,6 @@ AccordionContent._meta = {
   parent: 'Accordion',
 }
 
+AccordionContent.create = createShorthandFactory(AccordionContent, content => ({ content }))
+
 export default AccordionContent
diff --git a/src/modules/Accordion/AccordionTitle.js b/src/modules/Accordion/AccordionTitle.js
index 6762f7f99d..823f8be064 100644
--- a/src/modules/Accordion/AccordionTitle.js
+++ b/src/modules/Accordion/AccordionTitle.js
@@ -1,3 +1,4 @@
+import _ from 'lodash'
 import cx from 'classnames'
 import React, { Component, PropTypes } from 'react'
 
@@ -7,8 +8,11 @@ import {
   getUnhandledProps,
   META,
   useKeyOnly,
+  createShorthandFactory,
 } from '../../lib'
 
+import Icon from '../../elements/Icon'
+
 /**
  * A title sub-component for Accordion component.
  */
@@ -26,6 +30,9 @@ export default class AccordionTitle extends Component {
     /** Additional classes. */
     className: PropTypes.string,
 
+    /** Shorthand for primary content. */
+    content: customPropTypes.contentShorthand,
+
     /**
      * Called on click.
      *
@@ -52,6 +59,7 @@ export default class AccordionTitle extends Component {
       active,
       children,
       className,
+      content,
     } = this.props
 
     const classes = cx(
@@ -62,6 +70,21 @@ export default class AccordionTitle extends Component {
     const rest = getUnhandledProps(AccordionTitle, this.props)
     const ElementType = getElementType(AccordionTitle, this.props)
 
-    return <ElementType {...rest} className={classes} onClick={this.handleClick}>{children}</ElementType>
+    if (_.isNil(content)) {
+      return (
+        <ElementType {...rest} className={classes} onClick={this.handleClick}>
+          {children}
+        </ElementType>
+      )
+    }
+
+    return (
+      <ElementType {...rest} className={classes} onClick={this.handleClick}>
+        <Icon name='dropdown' />
+        {content}
+      </ElementType>
+    )
   }
 }
+
+AccordionTitle.create = createShorthandFactory(AccordionTitle, content => ({ content }))
diff --git a/src/modules/Accordion/index.d.ts b/src/modules/Accordion/index.d.ts
index fbbc3457e0..13e65b9a16 100644
--- a/src/modules/Accordion/index.d.ts
+++ b/src/modules/Accordion/index.d.ts
@@ -31,8 +31,9 @@ interface AccordionProps {
   onTitleClick?: (event: React.MouseEvent<HTMLDivElement>, index: number | number[]) => void;
 
   /**
-   * Create simple accordion panels from an array of { text: <string>, content: <custom> } objects.
+   * Create simple accordion panels from an array of { text: <custom>, content: <custom> } objects.
    * Object can optionally define an `active` key to open/close the panel.
+   * Object can opitonally define a `key` key used for title and content nodes' keys.
    * Mutually exclusive with children.
    */
   panels?: Array<any>;
diff --git a/test/specs/modules/Accordion/Accordion-test.js b/test/specs/modules/Accordion/Accordion-test.js
index 510f1ce110..c3a9b5ec05 100644
--- a/test/specs/modules/Accordion/Accordion-test.js
+++ b/test/specs/modules/Accordion/Accordion-test.js
@@ -242,9 +242,9 @@ describe('Accordion', () => {
         .should.not.have.descendants('#do-not-find-me')
     })
 
-    it('adds text and content sibling children', () => {
+    it('adds text title and text content sibling children', () => {
       const panels = [{
-        text: faker.lorem.sentence(),
+        title: faker.lorem.sentence(),
         content: faker.lorem.paragraph(),
       }]
       const wrapper = mount(<Accordion panels={panels} />)
@@ -254,22 +254,40 @@ describe('Accordion', () => {
         .should.have.className('title')
         .and.contain.text(panels[0].title)
 
+      expect(wrapper.childAt(0).key()).to.equal(`${panels[0].title}-title`)
+
       wrapper
         .childAt(1)
         .should.have.className('content')
         .and.contain.text(panels[0].content)
+
+      expect(wrapper.childAt(1).key()).to.equal(`${panels[0].title}-content`)
     })
 
-    it('allows setting the active prop', () => {
+    it('adds custom element title and custom element content sibling children', () => {
       const panels = [{
-        active: true,
-        title: faker.lorem.sentence(),
-        content: faker.lorem.paragraph(),
-      }, {
-        active: false,
-        title: faker.lorem.sentence(),
-        content: faker.lorem.paragraph(),
+        key: 'panel-1',
+        title: (<h1>{faker.lorem.sentence()}</h1>),
+        content: (<h2>{faker.lorem.paragraph()}</h2>),
       }]
+      const wrapper = mount(<Accordion panels={panels} />)
+
+      wrapper
+        .childAt(0)
+        .should.have.className('title')
+        .and.contain(panels[0].title)
+
+      expect(wrapper.childAt(0).key()).to.equal('panel-1-title')
+
+      wrapper
+        .childAt(1)
+        .should.have.className('content')
+        .and.contain(panels[0].content)
+
+      expect(wrapper.childAt(1).key()).to.equal('panel-1-content')
+    })
+
+    const checkIfAllowsSettingTheActiveProp = panels => {
       const wrapper = shallow(<Accordion panels={panels} />)
 
       // first panel (active)
@@ -295,12 +313,40 @@ describe('Accordion', () => {
         .find('AccordionContent')
         .at(1)
         .should.have.prop('active', false)
+    }
+
+    it('allows setting the active prop', () => {
+      const panels = [{
+        active: true,
+        title: faker.lorem.sentence(),
+        content: faker.lorem.paragraph(),
+      }, {
+        active: false,
+        title: faker.lorem.sentence(),
+        content: faker.lorem.paragraph(),
+      }]
+
+      checkIfAllowsSettingTheActiveProp(panels)
+    })
+
+    it('allows setting the active prop for custom title and content', () => {
+      const panels = [{
+        active: true,
+        title: (<div>faker.lorem.sentence()</div>),
+        content: (<p>faker.lorem.paragraph()</p>),
+      }, {
+        active: false,
+        title: (<h1>faker.lorem.sentence()</h1>),
+        content: (<h3>faker.lorem.paragraph()</h3>),
+      }]
+
+      checkIfAllowsSettingTheActiveProp(panels)
     })
 
     describe('onClick', () => {
       it('can be omitted', () => {
         const panels = [{
-          text: faker.lorem.sentence(),
+          title: faker.lorem.sentence(),
           content: faker.lorem.paragraph(),
         }]
         const wrapper = mount(<Accordion panels={panels} />)
diff --git a/test/specs/modules/Accordion/AccordionContent-test.js b/test/specs/modules/Accordion/AccordionContent-test.js
index b6d803f14e..ecd68f3e7a 100644
--- a/test/specs/modules/Accordion/AccordionContent-test.js
+++ b/test/specs/modules/Accordion/AccordionContent-test.js
@@ -5,4 +5,5 @@ describe('AccordionContent', () => {
   common.isConformant(AccordionContent)
   common.rendersChildren(AccordionContent)
   common.propKeyOnlyToClassName(AccordionContent, 'active')
+  common.implementsCreateMethod(AccordionContent)
 })
diff --git a/test/specs/modules/Accordion/AccordionTitle-test.js b/test/specs/modules/Accordion/AccordionTitle-test.js
index 125d0f18f2..7dcb1c960c 100644
--- a/test/specs/modules/Accordion/AccordionTitle-test.js
+++ b/test/specs/modules/Accordion/AccordionTitle-test.js
@@ -6,4 +6,5 @@ describe('AccordionTitle', () => {
   common.isConformant(AccordionTitle)
   common.rendersChildren(AccordionTitle)
   common.propKeyOnlyToClassName(AccordionTitle, 'active')
+  common.implementsCreateMethod(AccordionTitle)
 })