matt review

docs/src/pages/lab/breadcrumbs/CustomizedBreadcrumbs.js

@@ -1,5 +1,3 @@
-/* eslint-disable jsx-a11y/anchor-is-valid */
-
 import React from 'react';
 import PropTypes from 'prop-types';
 import { withStyles } from '@material-ui/core/styles';

docs/src/pages/lab/breadcrumbs/RouterBreadcrumbs.js

@@ -1,28 +1,46 @@
+/* eslint-disable no-nested-ternary */
+
 import React from 'react';
 import PropTypes from 'prop-types';
 import { withStyles } from '@material-ui/core/styles';
 import List from '@material-ui/core/List';
+import Link from '@material-ui/core/Link';
 import NoSsr from '@material-ui/core/NoSsr';
 import ListItem from '@material-ui/core/ListItem';
+import Collapse from '@material-ui/core/Collapse';
 import ListItemText from '@material-ui/core/ListItemText';
-import Typography from '@material-ui/core/Typography';
+import ExpandLess from '@material-ui/icons/ExpandLess';
+import ExpandMore from '@material-ui/icons/ExpandMore';
+import Breadcrumbs from '@material-ui/lab/Breadcrumbs';
+import Breadcrumb from '@material-ui/lab/Breadcrumb';
 import MemoryRouter from 'react-router/MemoryRouter';
 import Route from 'react-router/Route';
-import { Link } from 'react-router-dom';
+import { Link as RouterLink } from 'react-router-dom';
+
+const breadcrumbNameMap = {
+  '/inbox': 'Inbox',
+  '/inbox/important': 'Important',
+  '/trash': 'Trash',
+  '/spam': 'Spam',
+  '/drafts': 'Drafts',
+};
 
 function ListItemLink(props) {
-  const { primary, to } = props;
+  const { to, open, ...other } = props;
+  const primary = breadcrumbNameMap[to];
+
   return (
     <li>
-      <ListItem button component={Link} to={to} data-next="true">
+      <ListItem button component={RouterLink} to={to} data-next="true" {...other}>
         <ListItemText primary={primary} />
+        {open != null ? open ? <ExpandLess /> : <ExpandMore /> : null}
       </ListItem>
     </li>
   );
 }
 
 ListItemLink.propTypes = {
-  primary: PropTypes.node.isRequired,
+  open: PropTypes.bool,
   to: PropTypes.string.isRequired,
 };
 
@@ -34,32 +52,78 @@ const styles = theme => ({
   },
   lists: {
     backgroundColor: theme.palette.background.paper,
+    marginTop: theme.spacing.unit,
+  },
+  nested: {
+    paddingLeft: theme.spacing.unit * 4,
   },
 });
 
-function RouterBreadcrumbs(props) {
-  const { classes } = props;
+class RouterBreadcrumbs extends React.Component {
+  state = {
+    open: true,
+  };
 
-  // Use NoSsr to avoid SEO issues with the documentation website.
-  return (
-    <NoSsr>
-      <MemoryRouter initialEntries={['/drafts']} initialIndex={0}>
-        <div className={classes.root}>
-          <Route>
-            {({ location }) => (
-              <Typography gutterBottom>Current route: {location.pathname}</Typography>
-            )}
-          </Route>
-          <div className={classes.lists}>
-            <List component="nav">
-              <ListItemLink to="/trash" primary="Trash" />
-              <ListItemLink to="/spam" primary="Spam" />
-            </List>
+  handleClick = () => {
+    this.setState(state => ({ open: !state.open }));
+  };
+
+  render() {
+    const { classes } = this.props;
+
+    // Use NoSsr to avoid SEO issues with the documentation website.
+    return (
+      <NoSsr>
+        <MemoryRouter initialEntries={['/inbox']} initialIndex={0}>
+          <div className={classes.root}>
+            <Route>
+              {({ location }) => {
+                const pathnames = location.pathname.split('/').filter(x => x);
+
+                return (
+                  <Breadcrumbs arial-label="Breadcrumb navigation">
+                    <Breadcrumb>
+                      <Link component={RouterLink} color="inherit" to="/" data-next="true">
+                        Home
+                      </Link>
+                    </Breadcrumb>
+                    {pathnames.map((value, index) => {
+                      const last = index === pathnames.length - 1;
+                      const to = `/${pathnames.slice(0, index + 1).join('/')}`;
+
+                      return last ? (
+                        <Breadcrumb color="textPrimary" key={to}>
+                          {breadcrumbNameMap[to]}
+                        </Breadcrumb>
+                      ) : (
+                        <Breadcrumb key={to}>
+                          <Link component={RouterLink} color="inherit" to={to} data-next="true">
+                            {breadcrumbNameMap[to]}
+                          </Link>
+                        </Breadcrumb>
+                      );
+                    })}
+                  </Breadcrumbs>
+                );
+              }}
+            </Route>
+            <div className={classes.lists}>
+              <List component="nav">
+                <ListItemLink to="/inbox" open={this.state.open} onClick={this.handleClick} />
+                <Collapse in={this.state.open} timeout="auto" unmountOnExit>
+                  <List component="div" disablePadding>
+                    <ListItemLink to="/inbox/important" className={classes.nested} />
+                  </List>
+                </Collapse>
+                <ListItemLink to="/trash" />
+                <ListItemLink to="/spam" />
+              </List>
+            </div>
           </div>
-        </div>
-      </MemoryRouter>
-    </NoSsr>
-  );
+        </MemoryRouter>
+      </NoSsr>
+    );
+  }
 }
 
 RouterBreadcrumbs.propTypes = {

docs/src/pages/lab/breadcrumbs/breadcrumbs.md

@@ -27,7 +27,7 @@ components: Breadcrumbs, Breadcrumb
 
 If you have been reading the [overrides documentation page](/customization/overrides/)
 but you are not confident jumping in,
-here is one example of how you can change the badge position.
+here is one example of how you can change the breadcrumb link design.
 
 {{"demo": "pages/lab/breadcrumbs/CustomizedBreadcrumbs.js"}}
 

packages/material-ui-lab/src/Breadcrumb/Breadcrumb.d.ts

@@ -0,0 +1,11 @@
+import * as React from 'react';
+import { StandardProps } from '@material-ui/core';
+
+export interface BreadcrumbProps
+  extends StandardProps<React.HTMLAttributes<HTMLDivElement>, BreadcrumbClassKey> {}
+
+export type BreadcrumbClassKey = 'root';
+
+declare const Breadcrumb: React.ComponentType<BreadcrumbProps>;
+
+export default Breadcrumb;

packages/material-ui-lab/src/Breadcrumb/Breadcrumb.test.js

@@ -1,108 +1,29 @@
 import React from 'react';
 import { assert } from 'chai';
-import { spy } from 'sinon';
 import { createRender, createShallow, getClasses } from '@material-ui/core/test-utils';
-import ButtonBase from '@material-ui/core/ButtonBase';
-import Icon from '@material-ui/core/Icon';
+import Typography from '@material-ui/core/Typography';
 import Breadcrumb from './Breadcrumb';
 
 describe('<Breadcrumb />', () => {
   let shallow;
   let render;
   let classes;
-  const icon = <Icon>font_icon</Icon>;
 
   before(() => {
     shallow = createShallow({ dive: true });
     render = createRender();
-    classes = getClasses(<Breadcrumb label="Hello World" />);
+    classes = getClasses(<Breadcrumb />);
   });
 
-  it('should render a <ButtonBase> element', () => {
-    const wrapper = shallow(<Breadcrumb label="Hello World" />);
-    assert.strictEqual(wrapper.type(), ButtonBase);
+  it('should render a <Typography> element', () => {
+    const wrapper = shallow(<Breadcrumb />);
+    assert.strictEqual(wrapper.type(), Typography);
   });
 
   it('should render the root & gutter classes', () => {
-    const wrapper = shallow(<Breadcrumb label="Hello World" className="test-class-name" />);
+    const wrapper = shallow(<Breadcrumb className="test-class-name" />);
     assert.strictEqual(wrapper.is('.test-class-name'), true);
     assert.strictEqual(wrapper.hasClass(classes.root), true);
-    assert.strictEqual(wrapper.hasClass(classes.gutters), true);
-  });
-
-  it('should render the custom className and the root & gutter classes', () => {
-    const wrapper = shallow(<Breadcrumb label="Hello World" className="test-class-name" />);
-    assert.strictEqual(wrapper.is('.test-class-name'), true);
-    assert.strictEqual(wrapper.hasClass(classes.root), true);
-    assert.strictEqual(wrapper.hasClass(classes.gutters), true);
-  });
-
-  it('should render an Icon', () => {
-    const wrapper = shallow(<Breadcrumb label="Hello World" icon={icon} />);
-    const iconWrapper = wrapper.childAt(0);
-    assert.strictEqual(iconWrapper.find(Icon).length, 1);
-  });
-
-  it('should render the icon with the icon class', () => {
-    const wrapper = shallow(<Breadcrumb label="Hello World" icon={icon} />);
-    const iconWrapper = wrapper.childAt(0);
-    assert.strictEqual(iconWrapper.hasClass(classes.icon), true);
-  });
-
-  it('should not receive focus by default', () => {
-    const wrapper = shallow(<Breadcrumb label="Hello World" />);
-    assert.strictEqual(wrapper.props().tabIndex, -1);
-  });
-
-  describe('prop: active', () => {
-    let wrapper;
-    before(() => {
-      wrapper = shallow(<Breadcrumb label="Hello World" href="#" active />);
-    });
-
-    it('should render the breadcrumb with the active class', () => {
-      assert.strictEqual(wrapper.hasClass(classes.root), true);
-      assert.strictEqual(wrapper.hasClass(classes.gutters), true);
-      assert.strictEqual(wrapper.hasClass(classes.active), true);
-    });
-
-    it('should not receive focus', () => {
-      assert.strictEqual(wrapper.props().tabIndex, -1);
-    });
-  });
-
-  describe('prop: disableGutters', () => {
-    it('should render a breadcrumb without the gutters class', () => {
-      const wrapper = shallow(<Breadcrumb label="Hello World" disableGutters />);
-      assert.strictEqual(wrapper.hasClass(classes.root), true);
-      assert.strictEqual(wrapper.hasClass(classes.gutters), false);
-    });
-  });
-
-  describe('prop: href', () => {
-    it('should receive focus', () => {
-      const wrapper = shallow(<Breadcrumb label="Hello World" href="#" />);
-      assert.strictEqual(wrapper.props().tabIndex, 0);
-    });
-  });
-
-  describe('prop: onClick', () => {
-    let handleClick;
-    let wrapper;
-    before(() => {
-      handleClick = spy();
-      wrapper = shallow(<Breadcrumb onClick={handleClick} label="Hello World" />);
-    });
-
-    it('should be called when clicked', () => {
-      const event = {};
-      wrapper.simulate('click', event);
-      assert.strictEqual(handleClick.callCount, 1);
-    });
-
-    it('should receive focus', () => {
-      assert.strictEqual(wrapper.props().tabIndex, 0);
-    });
   });
 
   describe('server-side', () => {
@@ -112,7 +33,7 @@ describe('<Breadcrumb />', () => {
     }
 
     it('should server-side render', () => {
-      const markup = render(<Breadcrumb label="Hello World" />);
+      const markup = render(<Breadcrumb>Hello World</Breadcrumb>);
       assert.strictEqual(markup.text(), 'Hello World');
     });
   });

packages/material-ui-lab/src/Breadcrumbs/BreadcrumbSeparator.js

@@ -3,36 +3,33 @@ import PropTypes from 'prop-types';
 import classNames from 'classnames';
 import { withStyles } from '@material-ui/core/styles';
 
-const height = 24;
-
 const styles = theme => ({
   root: {
-    userSelect: 'none',
-    height,
-    lineHeight: `${height}px`,
     color: theme.palette.grey[400],
+    display: 'flex',
     marginLeft: 8,
     marginRight: 8,
+    userSelect: 'none',
   },
 });
 
 /**
  * @ignore - internal component.
  */
 function BreadcrumbSeparator(props) {
-  const { classes, className, separator, ...other } = props;
+  const { children, classes, className, ...other } = props;
 
   return (
     <li aria-hidden="true" className={classNames(classes.root, className)} {...other}>
-      {separator}
+      {children}
     </li>
   );
 }
 
 BreadcrumbSeparator.propTypes = {
+  children: PropTypes.node.isRequired,
   classes: PropTypes.object.isRequired,
   className: PropTypes.string,
-  separator: PropTypes.node.isRequired,
 };
 
 export default withStyles(styles, { name: 'MuiPrivateBreadcrumbSeparator' })(BreadcrumbSeparator);

packages/material-ui-lab/src/Breadcrumbs/BreadcrumbSeparator.test.js

@@ -9,17 +9,17 @@ describe('<BreadcrumbSeparator />', () => {
 
   before(() => {
     shallow = createShallow({ dive: true });
-    classes = getClasses(<BreadcrumbSeparator />);
+    classes = getClasses(<BreadcrumbSeparator>/</BreadcrumbSeparator>);
   });
 
-  it('should render a <div> element', () => {
-    const wrapper = shallow(<BreadcrumbSeparator />);
+  it('should render a <li> element', () => {
+    const wrapper = shallow(<BreadcrumbSeparator>/</BreadcrumbSeparator>);
 
-    assert.strictEqual(wrapper.type(), 'div');
+    assert.strictEqual(wrapper.type(), 'li');
   });
 
   it('should render the root class', () => {
-    const wrapper = shallow(<BreadcrumbSeparator />);
+    const wrapper = shallow(<BreadcrumbSeparator>/</BreadcrumbSeparator>);
 
     assert.strictEqual(wrapper.hasClass(classes.root), true);
   });

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.d.ts

@@ -0,0 +1,16 @@
+import * as React from 'react';
+import { StandardProps } from '@material-ui/core';
+
+export interface BreadcrumbsProps
+  extends StandardProps<React.HTMLAttributes<HTMLDivElement>, BreadcrumbsClassKey> {
+  itemsAfterCollapse?: boolean;
+  itemsBeforeCollapse?: boolean;
+  maxItems?: number;
+  separator?: React.ReactNode;
+}
+
+export type BreadcrumbsClassKey = 'root' | 'ol' | 'separator';
+
+declare const Breadcrumbs: React.ComponentType<BreadcrumbsProps>;
+
+export default Breadcrumbs;

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.js

@@ -41,9 +41,10 @@ class Breadcrumbs extends React.Component {
           <BreadcrumbSeparator
             // eslint-disable-next-line react/no-array-index-key
             key={`separator-${index}`}
-            separator={this.props.separator}
             className={this.props.classes.separator}
-          />,
+          >
+            {this.props.separator}
+          </BreadcrumbSeparator>,
         );
       } else {
         acc.push(current);