Merge pull request #20 from APMG/heading-levels-classes

Convert Heading component to separate level from className

package.json

@@ -45,7 +45,7 @@
     "@storybook/addon-info": "^5.0.3",
     "@storybook/addon-knobs": "^5.0.3",
     "@storybook/addon-options": "^5.0.3",
-    "@storybook/react": "^5.0.3",
+    "@storybook/react": "^5.0.11",
     "apm-html5-player": "^0.4.1",
     "babel-eslint": "^10.0.1",
     "babel-loader": "^8.0.5",
@@ -60,7 +60,7 @@
     "husky": "^1.3.1",
     "jest": "^24.5.0",
     "jest-prop-type-error": "^1.1.0",
-    "node-sass": "^4.11.0",
+    "node-sass": "^4.12.0",
     "prettier": "^1.16.4",
     "prop-types": "^15.7.2",
     "react-testing-library": "^6.0.0",

src/atoms/Heading/Heading.js

@@ -3,20 +3,29 @@ import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
 const Heading = (props) => {
+  // This component is for semantically providing h1 - h6
+  // depending on the html document outline.
+  // By default the `hdg-*` class will match the level,
+  // but to visually change the size, pass in classes
+  // through the `className` prop
+
   const classes = classNames({
-    hdg: true,
-    [`hdg-${props.level}`]: props.level,
+    hdg: !props.className,
+    [`hdg-${props.level}`]: !props.className && props.level,
+    [props.className]: props.className,
     [props.elementClass]: props.elementClass
   });
+
   const Hdg = `h${props.level}`;
 
   return <Hdg className={classes}>{props.children}</Hdg>;
 };
 
 Heading.propTypes = {
   children: PropTypes.node,
+  className: PropTypes.string,
   elementClass: PropTypes.string,
-  level: PropTypes.number.isRequired
+  level: PropTypes.oneOf([1, 2, 3, 4, 5, 6]).isRequired
 };
 
 export default Heading;

src/atoms/Heading/Heading.story.js

@@ -1,7 +1,6 @@
 import React from 'react';
-import { Link } from '@reach/router'; // eslint-disable-line
 import { storiesOf } from '@storybook/react';
-import { withKnobs, text, number } from '@storybook/addon-knobs';
+import { withKnobs, text, select } from '@storybook/addon-knobs';
 import Heading from './Heading';
 import { withReadme } from 'storybook-readme';
 import readme from './README.md';
@@ -13,10 +12,11 @@ stories.addDecorator(withKnobs()).addDecorator(withReadme([readme]));
 stories.add('Heading All Props', () => {
   return (
     <Heading
-      className={text('ClassName', 'hdg hdg-1')}
-      level={number('Heading Level', 1)}
+      className={text('className', '')}
+      elementClass={text('Element Class', '')}
+      level={select('Level', [1, 2, 3, 4, 5, 6], 1)}
     >
-      {text('Children', 'children')}
+      {text('Children', 'Heading Inner Text')}
     </Heading>
   );
 });

src/atoms/Heading/README.md

@@ -1,17 +1,21 @@
 # Heading
 
-Creates a generic `<h#>` DOM object.
+Creates a generic `<h#>` DOM object. By default the classes match the level, but ideally they should be passed in by using `className`. This is so that the semantic meaning of the heading can be separated from its appearance. E.g., sometimes we might want a heading to be very small stylistically (like an `h6`-equivalent size), but due to its location nested under an `<h2>` it must be an `<h3>`.
 
 ## Properties
 
 ### children
 
 Whatever you stick between the `<Heading>` tags.
 
+### className
+
+Accepts a string and replaces the default className of `Heading`. This should be used most of the time for applying the correct numbered `hdg hdg-*` classes.
+
 ### elementClass
 
-Accepts a string and adds that string to the className of `Heading`.
+Accepts a string and appends it to the end of the default/existing `className` of the heading.
 
-### level*
+### level\*
 
-Accepts a number from 1 to 6 and renders your header from `<h1>` to `<h6>` accordingly.
+Accepts a number from 1 to 6 and renders your header from `<h1>` to `<h6>` accordingly.

src/atoms/Heading/test/Heading.test.js

@@ -5,28 +5,58 @@ import Heading from '../Heading';
 // automatically unmount and cleanup DOM after the test is finished.
 afterEach(cleanup);
 
-describe('Heading tests', () => {
-  test('Heading level props "2", matches as expected', () => {
+describe('Heading', () => {
+  test('prop level={2} creates a <h2> element', () => {
+    const { getByText } = render(<Heading level={2}>children</Heading>);
+    expect(getByText('children').tagName).toBe('H2');
+  });
+
+  test('prop level={3} creates a <h3> element', () => {
+    const { getByText } = render(<Heading level={3}>children</Heading>);
+    expect(getByText('children').tagName).toBe('H3');
+  });
+
+  test('throws an error when a isRequired value is missing', () => {
+    expect(() => {
+      render(<Heading>children</Heading>);
+    }).toThrow();
+  });
+
+  test('throws an error if `level` outside 1-6 is supplied', () => {
+    expect(() => {
+      render(<Heading level={7}>children</Heading>);
+    }).toThrow();
+  });
+
+  test('sets a default className based on the heading level', () => {
+    const { getByText } = render(<Heading level={3}>children</Heading>);
+    expect(getByText('children').className).toBe('hdg hdg-3');
+  });
+
+  test('replaces the default class when a `className` prop is passed in', () => {
     const { getByText } = render(
-      <Heading title={'this is a title'} level={2}>
+      <Heading level={3} className="custom classes">
         children
       </Heading>
     );
-    expect(getByText('children').tagName).toBe('H2');
+    expect(getByText('children').className).toBe('custom classes');
   });
 
-  test('Heading level props "3", matches as expected', () => {
+  test('adds a class to the element when an `elementClass` prop is passed in', () => {
     const { getByText } = render(
-      <Heading title={'this is a title'} level={3}>
+      <Heading level={3} elementClass="custom classes">
         children
       </Heading>
     );
-    expect(getByText('children').tagName).toBe('H3');
+    expect(getByText('children').className).toBe('hdg hdg-3 custom classes');
   });
 
-  test('Throws an error when a isRequired value is missing', () => {
-    expect(() => {
-      render(<Heading title={'this is a title'} />);
-    }).toThrow();
+  test('allows both `className` replacement and appended `elementClass`', () => {
+    const { getByText } = render(
+      <Heading level={3} className="lots of" elementClass="custom classes">
+        children
+      </Heading>
+    );
+    expect(getByText('children').className).toBe('lots of custom classes');
   });
 });

src/molecules/EventTeaser/EventTeaser.js

@@ -14,7 +14,7 @@ const EventTeaser = (props) => {
             {props.title && (
               <Heading
                 level={props.headingLevel ? props.headingLevel : 1}
-                className="hdg hdg-4"
+                elementClass="hdg hdg-4"
               >
                 {props.title}
               </Heading>

src/molecules/EventTeaser/test/EventTeaser.test.js

@@ -75,12 +75,11 @@ describe('EventTeaser component', () => {
     expect(container.getElementsByClassName('eventInfo-venue').length).toBe(0);
   });
 
-  test('renders Heading if h1 prop exists', () => {
-    const { getByText, container } = render(
+  test('renders an h1 when prop headingLevel={1}', () => {
+    const { getByText } = render(
       <EventTeaser title={testProps.title} id={324} headingLevel={1} />
     );
     expect(getByText('this is a title').tagName).toBe('H1');
-    expect(container.getElementsByClassName('hdg-1').length).toBe(1);
   });
 
   test('throws an error when required `title` and` id` prop is missing', () => {

src/molecules/Teaser/Teaser.js

@@ -28,7 +28,7 @@ const Teaser = (props) => {
           <div className="teaser_header">
             <Heading
               level={props.headingLevel}
-              className={`hdg hdg-${props.headingLevel}`}
+              elementClass={`hdg hdg-${props.headingLevel}`}
             >
               {props.title}
             </Heading>