Merge pull request #134 from primer/underline-nav-fixes

UnderlineNav fixes

Author: shawnbot

package-lock.json

@@ -1,7 +1,6 @@
 import React from 'react'
 import PropTypes from 'prop-types'
 import classnames from 'classnames'
-import UnderlineNavLink from './UnderlineNavLink'
 import {mapWhitespaceProps} from './props'
 
 export const ITEM_CLASS = 'UnderlineNav-item no-underline'
@@ -13,25 +12,28 @@ export default function UnderlineNav(props) {
   const classes = classnames(className, 'UnderlineNav', align && `UnderlineNav--${align}`, full && 'UnderlineNav--full')
 
   const mappedChildren = React.Children.map(children, child => {
-    if (child.type === UnderlineNavLink) {
+    if (child.type.displayName === 'UnderlineNavLink') {
       return child
     }
 
-    const {className = '', selected} = child.props
+    let {className = ''} = child.props
     const newProps = {}
+
     // add the ITEM_CLASS to all children without one
     if (!className || !className.includes(ITEM_CLASS)) {
-      newProps.className = classnames(ITEM_CLASS, className)
+      className = classnames(ITEM_CLASS, className)
     }
-    if (selected === true && !className.includes(SELECTED_CLASS)) {
-      newProps.className = classnames(newProps.className || className, SELECTED_CLASS)
+
+    if (child.props.selected === true && !className.includes(SELECTED_CLASS)) {
+      className = classnames(className, SELECTED_CLASS)
     }
+
     // if this is a react-router NavLink (duck typing!),
     // set activeClassName={SELECTED_CLASS}
     if (child.type.name === 'NavLink') {
       newProps.activeClassName = SELECTED_CLASS
     }
-    return React.cloneElement(child, newProps)
+    return React.cloneElement(child, {className, ...newProps})
   })
 
   return (

src/UnderlineNav.js

@@ -16,6 +16,8 @@ export default function UnderlineNavLink(props) {
   )
 }
 
+UnderlineNavLink.displayName = 'UnderlineNavLink'
+
 UnderlineNavLink.defaultProps = {
   tag: 'a'
 }

src/UnderlineNavLink.js

@@ -1,9 +1,7 @@
 /* eslint-disable jsx-a11y/alt-text */
 import React from 'react'
 import CircleBadge from '../CircleBadge'
-import {render, renderClasses, mount} from '../utils/testing'
-
-const rendersClass = (node, klass) => renderClasses(node).includes(klass)
+import {render, rendersClass, mount} from '../utils/testing'
 
 const imgOutput = <img className="CircleBadge-icon" alt="" src="primer.jpg" />
 const imgInput = <img alt="" src="primer.jpg" />

src/__tests__/CircleBadge.js

@@ -1,6 +1,6 @@
 import React from 'react'
 import CircleOcticon from '../CircleOcticon'
-import {render, renderClasses} from '../utils/testing'
+import {render, rendersClass} from '../utils/testing'
 import {Check} from '@githubprimer/octicons-react'
 
 describe('CircleOcticon', () => {
@@ -9,23 +9,23 @@ describe('CircleOcticon', () => {
   })
 
   it('adds the "circle" class', () => {
-    expect(renderClasses(<CircleOcticon icon={Check} />).includes('circle')).toBe(true)
+    expect(rendersClass(<CircleOcticon icon={Check} />, 'circle')).toBe(true)
   })
 
   it('does not add a bg class by default', () => {
     expect(render(<CircleOcticon icon={Check} />).props.className).not.toMatch(/\bbg-/)
   })
 
   it('adds the appropriate bg class for the "bg" prop', () => {
-    expect(renderClasses(<CircleOcticon icon={Check} bg="red" />).includes('bg-red')).toBe(true)
+    expect(rendersClass(<CircleOcticon icon={Check} bg="red" />, 'bg-red')).toBe(true)
   })
 
   it('does not add a text class by default', () => {
     expect(render(<CircleOcticon icon={Check} />).props.className).not.toMatch(/\btext-/)
   })
 
   it('adds the appropriate text class for the "color" prop', () => {
-    expect(renderClasses(<CircleOcticon icon={Check} color="red" />).includes('text-red')).toBe(true)
+    expect(rendersClass(<CircleOcticon icon={Check} color="red" />, 'text-red')).toBe(true)
   })
 
   it('has a default size', () => {

src/__tests__/CircleOcticon.js

@@ -1,6 +1,6 @@
 import React from 'react'
 import Tooltip from '../Tooltip'
-import {render, renderClasses} from '../utils/testing'
+import {render, renderClasses, rendersClass} from '../utils/testing'
 
 describe('Tooltip', () => {
   it('renders a <span> with the "tooltipped" class', () => {
@@ -9,27 +9,27 @@ describe('Tooltip', () => {
   })
 
   it('respects the "align" prop', () => {
-    expect(renderClasses(<Tooltip align="ne" />).includes('tooltipped-align-ne-2')).toBe(true)
-    expect(renderClasses(<Tooltip align="se" />).includes('tooltipped-align-se-2')).toBe(true)
-    expect(renderClasses(<Tooltip align="nw" />).includes('tooltipped-align-nw-2')).toBe(true)
-    expect(renderClasses(<Tooltip align="sw" />).includes('tooltipped-align-sw-2')).toBe(true)
+    expect(rendersClass(<Tooltip align="ne" />, 'tooltipped-align-ne-2')).toBe(true)
+    expect(rendersClass(<Tooltip align="se" />, 'tooltipped-align-se-2')).toBe(true)
+    expect(rendersClass(<Tooltip align="nw" />, 'tooltipped-align-nw-2')).toBe(true)
+    expect(rendersClass(<Tooltip align="sw" />, 'tooltipped-align-sw-2')).toBe(true)
   })
 
   it('respects the "direction" prop', () => {
     for (const direction of Tooltip.directions) {
-      expect(renderClasses(<Tooltip direction={direction} />).includes(`tooltipped-${direction}`)).toBe(true)
+      expect(rendersClass(<Tooltip direction={direction} />, `tooltipped-${direction}`)).toBe(true)
     }
   })
 
   it('respects the "noDelay" prop', () => {
-    expect(renderClasses(<Tooltip noDelay />).includes(`tooltipped-no-delay`)).toBe(true)
+    expect(rendersClass(<Tooltip noDelay />, 'tooltipped-no-delay')).toBe(true)
   })
 
   it('respects the "text" prop', () => {
     expect(render(<Tooltip text="hi" />).props['aria-label']).toEqual('hi')
   })
 
   it('respects the "wrap" prop', () => {
-    expect(renderClasses(<Tooltip wrap />).includes('tooltipped-multiline')).toBe(true)
+    expect(rendersClass(<Tooltip wrap />, 'tooltipped-multiline')).toBe(true)
   })
 })

src/__tests__/Tooltip.js

@@ -2,9 +2,7 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import UnderlineNav, {ITEM_CLASS, SELECTED_CLASS} from '../UnderlineNav'
 import UnderlineNavLink from '../UnderlineNavLink'
-import {mount, render, renderClasses} from '../utils/testing'
-
-const rendersClass = (node, klass) => renderClasses(node).includes(klass)
+import {mount, render, rendersClass} from '../utils/testing'
 
 describe('Caret', () => {
   it('renders a <nav>', () => {

src/__tests__/UnderlineNav.js

@@ -4,6 +4,10 @@ import Adapter from 'enzyme-adapter-react-16'
 
 enzyme.configure({adapter: new Adapter()})
 
+export function mount(component) {
+  return enzyme.mount(component)
+}
+
 /**
  * Render the component (a React.createElement() or JSX expression)
  * into its intermediate object representation with 'type',
@@ -35,6 +39,6 @@ export function renderClasses(component) {
   return className ? className.trim().split(' ') : []
 }
 
-export function mount(component) {
-  return enzyme.mount(component)
+export function rendersClass(node, klass) {
+  return renderClasses(node).includes(klass)
 }