Repository: couds/react-bulma-components (js)

Issue:
Add Breadcrumb.Item and remove Breadcrumb items prop.
**Describe the bug**
Currently, `renderAs` prop affects the *items* of Breadcrumb instead of the component itself, which is not consistent with how it works on other components. Furthermore, props specified in the root component returned by the custom component passed to `renderAs` are not passed properly, so custom class names for example won't have any effect.

**To Reproduce**
Use the following code:

```jsx
const CustomComponent = ({ children, ...props }) => (
  // className is not passed to underlying Element
  <div className="other" {...props}>
    {children}
  </div>
)

const Nav = () => (
  <Breadcrumb
    renderAs={CustomComponent}
  />
)
```

**Proposed fix**
- `renderAs` to affect the Breadcrumb component itself
- new `itemRenderFunc` that is called for every item specified. The function is called with the item object itself.

cc @couds please let me know if this is expected behvaior.

Pull request description:
Add Breadcrumb.Item component to replace Breadcrumb items prop
Closes #290 
Documentation for Breadcrumb will be updated in #289 

Patch:
diff --git a/src/components/breadcrumb/breadcrumb.js b/src/components/breadcrumb/breadcrumb.js
index abce118..2a9f26a 100644
--- a/src/components/breadcrumb/breadcrumb.js
+++ b/src/components/breadcrumb/breadcrumb.js
@@ -1,18 +1,19 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import classnames from 'classnames';
+
+import Element from '../element';
+import BreadcrumbItem from './components/item';
 import modifiers from '../../modifiers';
 import renderAsShape from '../../modifiers/render-as';
-import Element from '../element';
 
 const Breadcrumb = ({
   className,
-  items,
   renderAs,
-  hrefAttr,
   separator,
   size,
   align,
+  children,
   ...props
 }) => (
   <Element
@@ -24,27 +25,12 @@ const Breadcrumb = ({
       [`is-${align}`]: align,
     })}
   >
-    <ul>
-      {items.map((item) => {
-        const p = {
-          renderAs,
-          [hrefAttr]: item.url,
-        };
-        return (
-          <li
-            key={item.url}
-            className={classnames({
-              'is-active': item.active,
-            })}
-          >
-            <Element {...p}>{item.name}</Element>
-          </li>
-        );
-      })}
-    </ul>
+    <ul>{children}</ul>
   </Element>
 );
 
+Breadcrumb.Item = BreadcrumbItem;
+
 Breadcrumb.propTypes = {
   ...modifiers.propTypes,
   className: PropTypes.string,
@@ -52,27 +38,19 @@ Breadcrumb.propTypes = {
   separator: PropTypes.oneOf(['arrow', 'bullet', 'dot', 'succeeds']),
   size: PropTypes.oneOf(['small', 'medium', 'large']),
   align: PropTypes.oneOf(['right', 'center']),
-  items: PropTypes.arrayOf(
-    PropTypes.shape({
-      url: PropTypes.string.isRequired,
-      active: PropTypes.bool,
-      name: PropTypes.node,
-    }),
-  ),
   renderAs: renderAsShape,
-  hrefAttr: PropTypes.string,
+  children: PropTypes.node,
 };
 
 Breadcrumb.defaultProps = {
   ...modifiers.defaultProps,
-  items: [],
-  hrefAttr: 'href',
   separator: undefined,
-  renderAs: 'a',
+  renderAs: 'nav',
   className: undefined,
   style: undefined,
   size: undefined,
   align: undefined,
+  children: undefined,
 };
 
 export default Breadcrumb;
diff --git a/src/components/breadcrumb/components/item.js b/src/components/breadcrumb/components/item.js
new file mode 100644
index 0000000..7fdf2e5
--- /dev/null
+++ b/src/components/breadcrumb/components/item.js
@@ -0,0 +1,38 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import classNames from 'classnames';
+
+import Element from '../../element';
+
+const BreadcrumbItem = ({ className, active, children, ...props }) => (
+  <Element
+    renderAs="li"
+    {...props}
+    className={classNames(className, {
+      'is-active': active,
+    })}
+  >
+    {children}
+  </Element>
+);
+
+BreadcrumbItem.propTypes = {
+  /**
+   * Additional CSS classes to pass to `<Breadcrumb.Item />`.
+   * They will sit alongside pre-applied bulma classes.
+   */
+  className: PropTypes.string,
+  /**
+   * Whether this item is the current active component in the breadcrumb
+   */
+  active: PropTypes.bool,
+  children: PropTypes.node,
+};
+
+BreadcrumbItem.defaultProps = {
+  className: undefined,
+  active: false,
+  children: undefined,
+};
+
+export default BreadcrumbItem;
diff --git a/src/components/breadcrumb/index.d.ts b/src/components/breadcrumb/index.d.ts
index f1721d8..6e68eb7 100644
--- a/src/components/breadcrumb/index.d.ts
+++ b/src/components/breadcrumb/index.d.ts
@@ -1,19 +1,16 @@
-import * as React from 'react';
 import { BulmaComponent } from '..';
 import { Size } from '../../modifiers';
 
-interface BreadcrumbItem {
-  url: string;
-  active?: boolean;
-  name?: React.ReactNode;
-}
-
 interface BreadcrumbProps {
   separator?: 'arrow' | 'bullet' | 'dot' | 'succeeds';
   size?: Size;
   align?: 'right' | 'center';
-  items?: BreadcrumbItem[];
-  hrefAttr?: string;
 }
 
-export const Breadcrumb: BulmaComponent<BreadcrumbProps, 'a'>;
+interface BreadcrumbItemProps {
+  active?: boolean;
+}
+
+export const Breadcrumb: BulmaComponent<BreadcrumbProps, 'nav'> & {
+  Item: BulmaComponent<BreadcrumbItemProps, 'li'>;
+};


Test patch:
diff --git a/src/components/breadcrumb/__test__/__snapshots__/breadcrumb.test.js.snap b/src/components/breadcrumb/__test__/__snapshots__/breadcrumb.test.js.snap
index d6ce432..1cb136b 100644
--- a/src/components/breadcrumb/__test__/__snapshots__/breadcrumb.test.js.snap
+++ b/src/components/breadcrumb/__test__/__snapshots__/breadcrumb.test.js.snap
@@ -1,31 +1,39 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Breadcrumb component Breadcrumb.Item component should have is-active class 1`] = `
+<li
+  className="is-active"
+>
+  active item
+</li>
+`;
+
+exports[`Breadcrumb component Breadcrumb.Item component should render <li> 1`] = `
+<li>
+  item
+</li>
+`;
+
 exports[`Breadcrumb component Should be a Breadcrumb 1`] = `
 <nav
   className="breadcrumb"
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="/"
       >
         Home
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="/section"
       >
         Section
       </a>
     </li>
-    <li
-      className="is-active"
-    >
+    <li>
       <a
         href="/detail"
       >
@@ -46,18 +54,14 @@ exports[`Breadcrumb component Should use inline style and custom size 1`] = `
   }
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="/"
       >
         Home
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="/section"
       >
@@ -82,27 +86,21 @@ exports[`Breadcrumb component should use separator arrow 1`] = `
   className="breadcrumb has-arrow-separator"
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#1"
       >
         Storybook
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#2"
       >
         Breadcrumb
       </a>
     </li>
-    <li
-      className="is-active"
-    >
+    <li>
       <a
         href="#3"
       >
@@ -118,27 +116,21 @@ exports[`Breadcrumb component should use separator bullet 1`] = `
   className="breadcrumb has-bullet-separator"
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#1"
       >
         Storybook
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#2"
       >
         Breadcrumb
       </a>
     </li>
-    <li
-      className="is-active"
-    >
+    <li>
       <a
         href="#3"
       >
@@ -154,27 +146,21 @@ exports[`Breadcrumb component should use separator dot 1`] = `
   className="breadcrumb has-dot-separator"
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#1"
       >
         Storybook
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#2"
       >
         Breadcrumb
       </a>
     </li>
-    <li
-      className="is-active"
-    >
+    <li>
       <a
         href="#3"
       >
@@ -190,27 +176,21 @@ exports[`Breadcrumb component should use separator null 1`] = `
   className="breadcrumb"
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#1"
       >
         Storybook
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#2"
       >
         Breadcrumb
       </a>
     </li>
-    <li
-      className="is-active"
-    >
+    <li>
       <a
         href="#3"
       >
@@ -226,27 +206,21 @@ exports[`Breadcrumb component should use separator succeeds 1`] = `
   className="breadcrumb has-succeeds-separator"
 >
   <ul>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#1"
       >
         Storybook
       </a>
     </li>
-    <li
-      className=""
-    >
+    <li>
       <a
         href="#2"
       >
         Breadcrumb
       </a>
     </li>
-    <li
-      className="is-active"
-    >
+    <li>
       <a
         href="#3"
       >
diff --git a/src/components/breadcrumb/__test__/breadcrumb.test.js b/src/components/breadcrumb/__test__/breadcrumb.test.js
index 9b15806..642416a 100644
--- a/src/components/breadcrumb/__test__/breadcrumb.test.js
+++ b/src/components/breadcrumb/__test__/breadcrumb.test.js
@@ -3,74 +3,71 @@ import renderer from 'react-test-renderer';
 import Breadcrumb from '..';
 
 describe('Breadcrumb component', () => {
+  describe('Breadcrumb.Item component', () => {
+    it('should render <li>', () => {
+      const component = renderer.create(
+        <Breadcrumb.Item>item</Breadcrumb.Item>,
+      );
+      expect(component.toJSON()).toMatchSnapshot();
+    });
+
+    it('should have is-active class', () => {
+      const component = renderer.create(
+        <Breadcrumb.Item active>active item</Breadcrumb.Item>,
+      );
+      expect(component.toJSON()).toMatchSnapshot();
+    });
+  });
+
   it('Should be a Breadcrumb', () => {
     const component = renderer.create(
-      <Breadcrumb
-        items={[
-          {
-            url: '/',
-            name: 'Home',
-          },
-          {
-            url: '/section',
-            name: 'Section',
-          },
-          {
-            url: '/detail',
-            name: 'Details',
-            active: true,
-          },
-        ]}
-      />,
+      <Breadcrumb>
+        <Breadcrumb.Item>
+          <a href="/">Home</a>
+        </Breadcrumb.Item>
+        <Breadcrumb.Item>
+          <a href="/section">Section</a>
+        </Breadcrumb.Item>
+        <Breadcrumb.Item>
+          <a href="/detail">Details</a>
+        </Breadcrumb.Item>
+      </Breadcrumb>,
     );
     expect(component.toJSON()).toMatchSnapshot();
   });
-  [null, 'arrow', 'dot', 'bullet', 'succeeds'].map(separator =>
+
+  [null, 'arrow', 'dot', 'bullet', 'succeeds'].map((separator) =>
     it(`should use separator ${separator}`, () => {
       const component = renderer.create(
-        <Breadcrumb
-          separator={separator}
-          items={[
-            {
-              name: 'Storybook',
-              url: '#1',
-            },
-            {
-              name: 'Breadcrumb',
-              url: '#2',
-            },
-            {
-              name: 'Breadcrumb Types',
-              url: '#3',
-              active: true,
-            },
-          ]}
-        />,
+        <Breadcrumb separator={separator}>
+          <Breadcrumb.Item>
+            <a href="#1">Storybook</a>
+          </Breadcrumb.Item>
+          <Breadcrumb.Item>
+            <a href="#2">Breadcrumb</a>
+          </Breadcrumb.Item>
+          <Breadcrumb.Item>
+            <a href="#3">Breadcrumb Types</a>
+          </Breadcrumb.Item>
+        </Breadcrumb>,
       );
       expect(component.toJSON()).toMatchSnapshot();
     }),
   );
+
   it('Should use inline style and custom size', () => {
     const component = renderer.create(
-      <Breadcrumb
-        style={{ marginTop: 10 }}
-        size="large"
-        items={[
-          {
-            url: '/',
-            name: 'Home',
-          },
-          {
-            url: '/section',
-            name: 'Section',
-          },
-          {
-            url: '/detail',
-            name: 'Details',
-            active: true,
-          },
-        ]}
-      />,
+      <Breadcrumb style={{ marginTop: 10 }} size="large">
+        <Breadcrumb.Item>
+          <a href="/">Home</a>
+        </Breadcrumb.Item>
+        <Breadcrumb.Item>
+          <a href="/section">Section</a>
+        </Breadcrumb.Item>
+        <Breadcrumb.Item active>
+          <a href="/detail">Details</a>
+        </Breadcrumb.Item>
+      </Breadcrumb>,
     );
     expect(component.toJSON()).toMatchSnapshot();
   });