[Breadcrumbs] Fix wrong role usage (#22366)

Co-authored-by: Olivier Tassinari <[email protected]> Co-authored-by: Sebastian Silbermann <[email protected]>
mui · Aug 27, 2020 · bdc85a8 · bdc85a8
1 parent ce073da
commit bdc85a8
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 29 deletions.
diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js
@@ -6,14 +6,13 @@ import MoreHorizIcon from '../internal/svg-icons/MoreHoriz';
 import ButtonBase from '../ButtonBase';
 
 const styles = (theme) => ({
-  root: {
+  button: {
     display: 'flex',
     marginLeft: theme.spacing(0.5),
     marginRight: theme.spacing(0.5),
     backgroundColor: theme.palette.grey[100],
     color: theme.palette.grey[700],
     borderRadius: 2,
-    cursor: 'pointer',
     '&:hover, &:focus': {
       backgroundColor: theme.palette.grey[200],
     },
@@ -35,9 +34,11 @@ function BreadcrumbCollapsed(props) {
   const { classes, ...other } = props;
 
   return (
-    <ButtonBase component="li" className={classes.root} focusRipple {...other}>
-      <MoreHorizIcon className={classes.icon} />
-    </ButtonBase>
+    <li>
+      <ButtonBase className={classes.button} focusRipple {...other}>
+        <MoreHorizIcon className={classes.icon} />
+      </ButtonBase>
+    </li>
   );
 }
 

diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js
@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { spy } from 'sinon';
-import { getClasses, act, fireEvent, createClientRender } from 'test/utils';
+import { getClasses, fireEvent, createClientRender } from 'test/utils';
 import BreadcrumbCollapsed from './BreadcrumbCollapsed';
 
 describe('<BreadcrumbCollapsed />', () => {
@@ -13,35 +13,25 @@ describe('<BreadcrumbCollapsed />', () => {
   });
 
   it('should render an icon', () => {
-    const { container } = render(<BreadcrumbCollapsed />);
+    const { container, getByRole } = render(<BreadcrumbCollapsed />);
 
     expect(container.querySelectorAll('svg').length).to.equal(1);
-    expect(container.firstChild).to.have.class(classes.root);
+    expect(getByRole('button')).to.have.class(classes.button);
   });
 
-  describe('prop: onKeyDown', () => {
-    it(`should be called on key down - Enter`, () => {
-      const handleClick = spy();
-      const { container } = render(<BreadcrumbCollapsed onClick={handleClick} />);
-      const expand = container.firstChild;
-
-      act(() => {
-        expand.focus();
-        fireEvent.keyDown(expand, { key: 'Enter' });
-      });
+  it('renders a native <button>', () => {
+    const { getByRole } = render(<BreadcrumbCollapsed />);
 
-      expect(handleClick.callCount).to.equal(1);
-    });
+    expect(getByRole('button')).to.have.property('nodeName', 'BUTTON');
+  });
 
-    it(`should be called on key up - Space`, () => {
+  describe('prop: onClick', () => {
+    it(`should be called when clicked`, () => {
       const handleClick = spy();
-      const { container } = render(<BreadcrumbCollapsed onClick={handleClick} />);
-      const expand = container.firstChild;
+      const { getByRole } = render(<BreadcrumbCollapsed onClick={handleClick} />);
+      const expand = getByRole('button');
 
-      act(() => {
-        expand.focus();
-        fireEvent.keyUp(expand, { key: ' ' });
-      });
+      fireEvent.click(expand);
 
       expect(handleClick.callCount).to.equal(1);
     });

diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
@@ -68,7 +68,10 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {
 
       // The clicked element received the focus but gets removed from the DOM.
       // Let's keep the focus in the component after expanding.
-      const focusable = event.currentTarget.parentNode.querySelector('a[href],button,[tabindex]');
+      const focusable = event.currentTarget.parentNode.parentNode.querySelector(
+        'a[href],button,[tabindex]',
+      );
+
       if (focusable) {
         focusable.focus();
       }

diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js
@@ -60,7 +60,7 @@ describe('<Breadcrumbs />', () => {
 
     const listitems = getAllByRole('listitem', { hidden: false });
 
-    expect(listitems).to.have.length(2);
+    expect(listitems).to.have.length(3);
     expect(getByRole('list')).to.have.text('first//ninth');
     expect(getByRole('button').querySelector('[data-mui-test="MoreHorizIcon"]')).not.to.equal(null);
   });