feat(ui-breadcrumb): add icon support

This is a follow-up commit to https://gerrit.instructure.com/164847,
which only works with links, not buttons (this was an oversight). This
commit passes the icon through to the underlying Link component for
display.

Refs INSTUI-1543

Test plan: check that all Breadcrumb examples look good

Change-Id: Idb9fc3a54977d58a19b613a31e800cc9fb5ff3ef
Reviewed-on: https://gerrit.instructure.com/164975
Tested-by: Jenkins
Reviewed-by: Omar Khan <okhan@instructure.com>
Reviewed-by: Ken Meleta <kmeleta@instructure.com>
Product-Review: Ken Meleta <kmeleta@instructure.com>
QA-Review: Dan Sasaki <dsasaki@instructure.com>

Author: omarkhan

packages/ui-breadcrumb/src/components/Breadcrumb/BreadcrumbLink/__tests__/BreadcrumbLink.test.js

@@ -23,18 +23,15 @@
  */
 
 import React from 'react'
-import Link from '@instructure/ui-elements/lib/components/Link'
 
 import BreadcrumbLink from '../index'
 
-import styles from '../styles.css'
-
 describe('<BreadcrumbLink />', () => {
   const testbed = new Testbed(<BreadcrumbLink>Content</BreadcrumbLink>)
 
-  it('should render a Link component when given an href prop', () => {
+  it('should render a link when given an href prop', () => {
     const subject = testbed.render({href: '#'})
-    expect(subject.find(Link)).to.be.present()
+    expect(subject.find('a')).to.be.present()
   })
 
   it('should render as a button and respond to onClick event', () => {
@@ -44,9 +41,14 @@ describe('<BreadcrumbLink />', () => {
     expect(onClick).to.have.been.called()
   })
 
-  it('should not render a Link component when not given an href prop', () => {
+  it('should not render a link when not given an href prop', () => {
+    const subject = testbed.render()
+    expect(subject.find('a')).not.to.be.present()
+  })
+
+  it('should not render a button when not given an onClick prop', () => {
     const subject = testbed.render()
-    expect(subject.find(`.${styles.text}`)).to.be.present()
+    expect(subject.find('button')).not.to.be.present()
   })
 
   it('should meet a11y standards', (done) => {

packages/ui-breadcrumb/src/components/Breadcrumb/BreadcrumbLink/index.js

@@ -25,18 +25,13 @@
 import React, { Component } from 'react'
 import PropTypes from 'prop-types'
 import Link from '@instructure/ui-elements/lib/components/Link'
-import themeable from '@instructure/ui-themeable'
 import { omitProps } from '@instructure/ui-utils/lib/react/passthroughProps'
 
-import styles from './styles.css'
-import theme from './theme'
-
 /**
 ---
 parent: Breadcrumb
 ---
 **/
-@themeable(theme, styles)
 export default class BreadcrumbLink extends Component {
   static propTypes = {
     /**
@@ -54,35 +49,44 @@ export default class BreadcrumbLink extends Component {
     /**
     * Sets the font-size of the breadcrumb text
     */
-    size: PropTypes.oneOf(['small', 'medium', 'large'])
+    size: PropTypes.oneOf(['small', 'medium', 'large']),
+    /**
+    * Add an icon to the BreadcrumbLink
+    */
+    icon: PropTypes.oneOfType([PropTypes.func, PropTypes.element]),
+    /**
+    * Place the icon before or after the text in the BreadcrumbLink
+    */
+    iconPlacement: PropTypes.oneOf(['start', 'end'])
   }
 
   render () {
     const {
       children,
       href,
+      icon,
+      iconPlacement,
       onClick
     } = this.props
 
     const props = omitProps(this.props, BreadcrumbLink.propTypes)
 
-    if (href || onClick) {
-      return (
-        <Link
-          {...props}
-          ellipsis
-          href={href}
-          onClick={onClick}
-        >
-          {children}
-        </Link>
-      )
-    } else {
-      return (
-        <span className={styles.text}>
-          <span className={styles.ellipsis}>{children}</span>
-        </span>
-      )
-    }
+    // Link will display a button by default, even if there is no action.
+    // Force a span in this case.
+    const as = href || onClick ? void 0 : 'span'
+
+    return (
+      <Link
+        as={as}
+        {...props}
+        ellipsis
+        href={href}
+        icon={icon}
+        iconPlacement={iconPlacement}
+        onClick={onClick}
+      >
+        {children}
+      </Link>
+    )
   }
 }

packages/ui-breadcrumb/src/components/Breadcrumb/BreadcrumbLink/styles.css

@@ -68,18 +68,16 @@ example: true
 </Breadcrumb>
 ```
 
-You can include icons inside `BreadcrumbLink`:
+You can include icons in `BreadcrumbLink`:
 
 ```js
 ---
 example: true
 ---
-<Breadcrumb label="You are here:">
-  <BreadcrumbLink href="https://instructure.github.io/instructure-ui/">
-    <IconBank.Line size="small" /> Item Bank
-  </BreadcrumbLink>
-  <BreadcrumbLink href="https://instructure.github.io/instructure-ui/">History</BreadcrumbLink>
-  <BreadcrumbLink>Question</BreadcrumbLink>
+<Breadcrumb label="Breadcrumb with icons">
+  <BreadcrumbLink icon={<IconBank.Line size="small" />} href="#Breadcrumb">Item Bank</BreadcrumbLink>
+  <BreadcrumbLink icon={<IconClock.Line size="small" />} onClick={() => {}}>History</BreadcrumbLink>
+  <BreadcrumbLink icon={IconPlus.Line} iconPlacement="end">New Question</BreadcrumbLink>
 </Breadcrumb>
 ```
 
@@ -100,4 +98,3 @@ guidelines: true
   </Figure>
 </Guidelines>
 ```
-

packages/ui-breadcrumb/src/components/Breadcrumb/BreadcrumbLink/theme.js

@@ -31,6 +31,7 @@ import IconArrowOpenEnd from '@instructure/ui-icons/lib/Solid/IconArrowOpenEnd'
 import themeable from '@instructure/ui-themeable'
 import ThemeablePropTypes from '@instructure/ui-themeable/lib/utils/ThemeablePropTypes'
 import CustomPropTypes from '@instructure/ui-utils/lib/react/CustomPropTypes'
+import Browser from '@instructure/ui-utils/lib/Browser'
 
 import BreadcrumbLink from './BreadcrumbLink'
 
@@ -88,10 +89,16 @@ export default class Breadcrumb extends Component {
   }
 
   render () {
+    // FF and EDGE calculate flex baseline alignment differently, and do better with `center`
+    // in this case: http://kizu.ru/en/blog/flex-baseline/#another-way
+    const preferAlignCenter = Browser.firefox || Browser.msedge
+
     const classes = {
       [styles.root]: true,
+      [styles.preferAlignCenter]: preferAlignCenter,
       [styles[this.props.size]]: true
     }
+
     return (
       <View
         role="navigation"

packages/ui-breadcrumb/src/components/Breadcrumb/README.md

@@ -1,49 +1,71 @@
 /* stylelint-disable selector-max-class */
 
 .root {
-  display: flex;
-  flex-direction: row;
-  align-items: baseline;
+  font-family: var(--fontFamily);
   margin: 0;
   padding: 0;
   list-style-type: none;
   overflow: visible;
+  display: flex;
+  align-items: baseline;
+}
+
+.root.preferAlignCenter {
+  align-items: center;
 }
 
 .crumb {
-  display: flex;
-  flex-direction: row;
-  align-items: baseline;
+  box-sizing: border-box;
+  position: relative;
+}
+
+.crumb:last-child {
+  padding-inline-end: 0;
 }
 
 .separator {
+  box-sizing: border-box;
+  position: absolute;
+  top: 50%;
   color: var(--separatorColor);
 }
 
 .small .crumb {
   font-size: var(--smallFontSize);
+  padding-inline-end: calc(var(--smallSeparatorFontSize) * 2);
+  padding-inline-start: 0;
 
   .separator {
     font-size: var(--smallSeparatorFontSize);
-    margin: 0 calc(var(--smallSeparatorFontSize) / 2);
+    offset-inline-end: calc(var(--smallSeparatorFontSize) / 2);
+    offset-inline-start: auto;
+    margin-top: calc(-1 * (var(--smallSeparatorFontSize) / 2));
   }
 }
 
 .medium .crumb {
   font-size: var(--mediumFontSize);
+  padding-inline-end: calc(var(--mediumSeparatorFontSize) * 2);
+  padding-inline-start: 0;
 
   .separator {
     font-size: var(--mediumSeparatorFontSize);
-    margin: 0 calc(var(--mediumSeparatorFontSize) / 2);
+    offset-inline-end: calc(var(--mediumSeparatorFontSize) / 2);
+    offset-inline-start: auto;
+    margin-top: calc(-1 * (var(--mediumSeparatorFontSize) / 2));
   }
 }
 
 .large .crumb {
   font-size: var(--largeFontSize);
+  padding-inline-end: calc(var(--largeSeparatorFontSize) * 2);
+  padding-inline-start: 0;
 
   .separator {
     font-size: var(--largeSeparatorFontSize);
-    margin: 0 calc(var(--largeSeparatorFontSize) / 2);
+    offset-inline-end: calc(var(--largeSeparatorFontSize) / 2);
+    offset-inline-start: auto;
+    margin-top: calc(-1 * (var(--largeSeparatorFontSize) / 2));
   }
 }
 

packages/ui-breadcrumb/src/components/Breadcrumb/index.js

@@ -24,6 +24,7 @@
 
 export default function generator ({ colors, typography }) {
   return {
+    fontFamily: typography.fontFamily,
     separatorColor: colors.borderDark,
 
     smallSeparatorFontSize: '0.5rem',

packages/ui-breadcrumb/src/components/Breadcrumb/styles.css

@@ -47,6 +47,7 @@
   }
 }
 
+.root .inverse,
 .root a.inverse:link,
 .root a.inverse:visited,
 .root button.inverse {
@@ -79,18 +80,21 @@
   font-size: 1em;
   margin: 0;
   padding: 0;
-  display: inline-flex;
-  align-content: flex-start;
-  text-align: inherit;
 }
 
 .ellipsis {
+  box-sizing: border-box;
   display: block;
   overflow: hidden;
   white-space: nowrap;
   text-overflow: ellipsis;
 }
 
+button.ellipsis {
+  width: 100%;
+  text-align: start;
+}
+
 .icon {
   font-size: 1.125em; /* make icon slightly larger than inherited font-size */
 }