From 89d7ad0a4274b721f3593a631b000ab02a44d6c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Artur=20Bie=C5=84?= <artur.bien@gmail.com>
Date: Fri, 10 Apr 2020 23:18:41 +0200
Subject: [PATCH] [Table] Add role if the default role of elements can't be
 used (#20475)

---
 packages/material-ui/src/Table/Table.js       |  5 ++-
 packages/material-ui/src/Table/Table.test.js  |  5 +++
 .../material-ui/src/TableBody/TableBody.js    | 11 +++++--
 .../src/TableBody/TableBody.test.js           | 16 +++++++++-
 .../material-ui/src/TableCell/TableCell.js    |  8 +++--
 .../src/TableFooter/TableFooter.js            | 11 +++++--
 .../src/TableFooter/TableFooter.test.js       | 16 +++++++++-
 .../material-ui/src/TableHead/TableHead.js    | 11 +++++--
 .../src/TableHead/TableHead.test.js           | 16 +++++++++-
 packages/material-ui/src/TableRow/TableRow.js |  4 ++-
 .../material-ui/src/TableRow/TableRow.test.js | 16 +++++++++-
 .../test/integration/TableCell.test.js        | 32 ++++++++++++++++++-
 12 files changed, 136 insertions(+), 15 deletions(-)

diff --git a/packages/material-ui/src/Table/Table.js b/packages/material-ui/src/Table/Table.js
index c8606cb774d22d..f2e31b1ec6e6b5 100644
--- a/packages/material-ui/src/Table/Table.js
+++ b/packages/material-ui/src/Table/Table.js
@@ -25,11 +25,13 @@ export const styles = (theme) => ({
   },
 });
 
+const defaultComponent = 'table';
+
 const Table = React.forwardRef(function Table(props, ref) {
   const {
     classes,
     className,
-    component: Component = 'table',
+    component: Component = defaultComponent,
     padding = 'default',
     size = 'medium',
     stickyHeader = false,
@@ -44,6 +46,7 @@ const Table = React.forwardRef(function Table(props, ref) {
   return (
     <TableContext.Provider value={table}>
       <Component
+        role={Component === defaultComponent ? null : 'table'}
         ref={ref}
         className={clsx(classes.root, { [classes.stickyHeader]: stickyHeader }, className)}
         {...other}
diff --git a/packages/material-ui/src/Table/Table.test.js b/packages/material-ui/src/Table/Table.test.js
index a7c69f74dc0557..a7d5f2532c3517 100644
--- a/packages/material-ui/src/Table/Table.test.js
+++ b/packages/material-ui/src/Table/Table.test.js
@@ -36,6 +36,11 @@ describe('<Table />', () => {
       const { container } = render(<Table component="div">foo</Table>);
       expect(container.firstChild).to.have.property('nodeName', 'DIV');
     });
+
+    it('sets role="table"', () => {
+      const { container } = render(<Table component="div">foo</Table>);
+      expect(container.firstChild).to.have.attribute('role', 'table');
+    });
   });
 
   it('should render children', () => {
diff --git a/packages/material-ui/src/TableBody/TableBody.js b/packages/material-ui/src/TableBody/TableBody.js
index 470c24148acca0..50ebdbcdf98069 100644
--- a/packages/material-ui/src/TableBody/TableBody.js
+++ b/packages/material-ui/src/TableBody/TableBody.js
@@ -15,12 +15,19 @@ const tablelvl2 = {
   variant: 'body',
 };
 
+const defaultComponent = 'tbody';
+
 const TableBody = React.forwardRef(function TableBody(props, ref) {
-  const { classes, className, component: Component = 'tbody', ...other } = props;
+  const { classes, className, component: Component = defaultComponent, ...other } = props;
 
   return (
     <Tablelvl2Context.Provider value={tablelvl2}>
-      <Component className={clsx(classes.root, className)} ref={ref} {...other} />
+      <Component
+        className={clsx(classes.root, className)}
+        ref={ref}
+        role={Component === defaultComponent ? null : 'rowgroup'}
+        {...other}
+      />
     </Tablelvl2Context.Provider>
   );
 });
diff --git a/packages/material-ui/src/TableBody/TableBody.test.js b/packages/material-ui/src/TableBody/TableBody.test.js
index 864b8ec93999ce..637782f5a8e3aa 100644
--- a/packages/material-ui/src/TableBody/TableBody.test.js
+++ b/packages/material-ui/src/TableBody/TableBody.test.js
@@ -1,13 +1,15 @@
 import * as React from 'react';
-import { assert } from 'chai';
+import { assert, expect } from 'chai';
 import { createMount, getClasses } from '@material-ui/core/test-utils';
 import describeConformance from '../test-utils/describeConformance';
+import { createClientRender } from 'test/utils/createClientRender';
 import TableBody from './TableBody';
 import Tablelvl2Context from '../Table/Tablelvl2Context';
 
 describe('<TableBody />', () => {
   let mount;
   let classes;
+  const render = createClientRender();
 
   function mountInTable(node) {
     const wrapper = mount(<table>{node}</table>);
@@ -53,4 +55,16 @@ describe('<TableBody />', () => {
     );
     assert.strictEqual(context.variant, 'body');
   });
+
+  describe('prop: component', () => {
+    it('can render a different component', () => {
+      const { container } = render(<TableBody component="div" />);
+      expect(container.firstChild).to.have.property('nodeName', 'DIV');
+    });
+
+    it('sets role="rowgroup"', () => {
+      const { container } = render(<TableBody component="div" />);
+      expect(container.firstChild).to.have.attribute('role', 'rowgroup');
+    });
+  });
 });
diff --git a/packages/material-ui/src/TableCell/TableCell.js b/packages/material-ui/src/TableCell/TableCell.js
index a9b417979cf4c8..da3ec5e813ce06 100644
--- a/packages/material-ui/src/TableCell/TableCell.js
+++ b/packages/material-ui/src/TableCell/TableCell.js
@@ -122,15 +122,18 @@ const TableCell = React.forwardRef(function TableCell(props, ref) {
   const table = React.useContext(TableContext);
   const tablelvl2 = React.useContext(Tablelvl2Context);
 
+  const isHeadCell = tablelvl2 && tablelvl2.variant === 'head';
+  let role;
   let Component;
   if (component) {
     Component = component;
+    role = isHeadCell ? 'columnheader' : 'cell';
   } else {
-    Component = tablelvl2 && tablelvl2.variant === 'head' ? 'th' : 'td';
+    Component = isHeadCell ? 'th' : 'td';
   }
 
   let scope = scopeProp;
-  if (!scope && tablelvl2 && tablelvl2.variant === 'head') {
+  if (!scope && isHeadCell) {
     scope = 'col';
   }
   const padding = paddingProp || (table && table.padding ? table.padding : 'default');
@@ -157,6 +160,7 @@ const TableCell = React.forwardRef(function TableCell(props, ref) {
         className,
       )}
       aria-sort={ariaSort}
+      role={role}
       scope={scope}
       {...other}
     />
diff --git a/packages/material-ui/src/TableFooter/TableFooter.js b/packages/material-ui/src/TableFooter/TableFooter.js
index 7ac47289a4aa94..d68c1a4d3642bd 100644
--- a/packages/material-ui/src/TableFooter/TableFooter.js
+++ b/packages/material-ui/src/TableFooter/TableFooter.js
@@ -15,12 +15,19 @@ const tablelvl2 = {
   variant: 'footer',
 };
 
+const defaultComponent = 'tfoot';
+
 const TableFooter = React.forwardRef(function TableFooter(props, ref) {
-  const { classes, className, component: Component = 'tfoot', ...other } = props;
+  const { classes, className, component: Component = defaultComponent, ...other } = props;
 
   return (
     <Tablelvl2Context.Provider value={tablelvl2}>
-      <Component className={clsx(classes.root, className)} ref={ref} {...other} />
+      <Component
+        className={clsx(classes.root, className)}
+        ref={ref}
+        role={Component === defaultComponent ? null : 'rowgroup'}
+        {...other}
+      />
     </Tablelvl2Context.Provider>
   );
 });
diff --git a/packages/material-ui/src/TableFooter/TableFooter.test.js b/packages/material-ui/src/TableFooter/TableFooter.test.js
index f79ee4113fefcb..a3b7499c06bbb4 100644
--- a/packages/material-ui/src/TableFooter/TableFooter.test.js
+++ b/packages/material-ui/src/TableFooter/TableFooter.test.js
@@ -1,13 +1,15 @@
 import * as React from 'react';
-import { assert } from 'chai';
+import { assert, expect } from 'chai';
 import { createMount, getClasses } from '@material-ui/core/test-utils';
 import describeConformance from '../test-utils/describeConformance';
+import { createClientRender } from 'test/utils/createClientRender';
 import TableFooter from './TableFooter';
 import Tablelvl2Context from '../Table/Tablelvl2Context';
 
 describe('<TableFooter />', () => {
   let mount;
   let classes;
+  const render = createClientRender();
 
   function mountInTable(node) {
     const wrapper = mount(<table>{node}</table>);
@@ -51,4 +53,16 @@ describe('<TableFooter />', () => {
     );
     assert.strictEqual(context.variant, 'footer');
   });
+
+  describe('prop: component', () => {
+    it('can render a different component', () => {
+      const { container } = render(<TableFooter component="div" />);
+      expect(container.firstChild).to.have.property('nodeName', 'DIV');
+    });
+
+    it('sets role="rowgroup"', () => {
+      const { container } = render(<TableFooter component="div" />);
+      expect(container.firstChild).to.have.attribute('role', 'rowgroup');
+    });
+  });
 });
diff --git a/packages/material-ui/src/TableHead/TableHead.js b/packages/material-ui/src/TableHead/TableHead.js
index e0f83acf3dcb2b..beec5ace7c8b4f 100644
--- a/packages/material-ui/src/TableHead/TableHead.js
+++ b/packages/material-ui/src/TableHead/TableHead.js
@@ -15,12 +15,19 @@ const tablelvl2 = {
   variant: 'head',
 };
 
+const defaultComponent = 'thead';
+
 const TableHead = React.forwardRef(function TableHead(props, ref) {
-  const { classes, className, component: Component = 'thead', ...other } = props;
+  const { classes, className, component: Component = defaultComponent, ...other } = props;
 
   return (
     <Tablelvl2Context.Provider value={tablelvl2}>
-      <Component className={clsx(classes.root, className)} ref={ref} {...other} />
+      <Component
+        className={clsx(classes.root, className)}
+        ref={ref}
+        role={Component === defaultComponent ? null : 'rowgroup'}
+        {...other}
+      />
     </Tablelvl2Context.Provider>
   );
 });
diff --git a/packages/material-ui/src/TableHead/TableHead.test.js b/packages/material-ui/src/TableHead/TableHead.test.js
index 8eb32198d8de8b..37b172e8e4ade1 100644
--- a/packages/material-ui/src/TableHead/TableHead.test.js
+++ b/packages/material-ui/src/TableHead/TableHead.test.js
@@ -1,13 +1,15 @@
 import * as React from 'react';
-import { assert } from 'chai';
+import { assert, expect } from 'chai';
 import { createMount, getClasses } from '@material-ui/core/test-utils';
 import describeConformance from '../test-utils/describeConformance';
+import { createClientRender } from 'test/utils/createClientRender';
 import TableHead from './TableHead';
 import Tablelvl2Context from '../Table/Tablelvl2Context';
 
 describe('<TableHead />', () => {
   let mount;
   let classes;
+  const render = createClientRender();
   function mountInTable(node) {
     const wrapper = mount(<table>{node}</table>);
     return wrapper.find('table').childAt(0);
@@ -50,4 +52,16 @@ describe('<TableHead />', () => {
     );
     assert.strictEqual(context.variant, 'head');
   });
+
+  describe('prop: component', () => {
+    it('can render a different component', () => {
+      const { container } = render(<TableHead component="div" />);
+      expect(container.firstChild).to.have.property('nodeName', 'DIV');
+    });
+
+    it('sets role="rowgroup"', () => {
+      const { container } = render(<TableHead component="div" />);
+      expect(container.firstChild).to.have.attribute('role', 'rowgroup');
+    });
+  });
 });
diff --git a/packages/material-ui/src/TableRow/TableRow.js b/packages/material-ui/src/TableRow/TableRow.js
index 0fb9458c72999e..0e3517e47fd097 100644
--- a/packages/material-ui/src/TableRow/TableRow.js
+++ b/packages/material-ui/src/TableRow/TableRow.js
@@ -30,6 +30,7 @@ export const styles = (theme) => ({
   footer: {},
 });
 
+const defaultComponent = 'tr';
 /**
  * Will automatically set dynamic row height
  * based on the material table element parent (head, body, etc).
@@ -38,7 +39,7 @@ const TableRow = React.forwardRef(function TableRow(props, ref) {
   const {
     classes,
     className,
-    component: Component = 'tr',
+    component: Component = defaultComponent,
     hover = false,
     selected = false,
     ...other
@@ -58,6 +59,7 @@ const TableRow = React.forwardRef(function TableRow(props, ref) {
         },
         className,
       )}
+      role={Component === defaultComponent ? null : 'row'}
       {...other}
     />
   );
diff --git a/packages/material-ui/src/TableRow/TableRow.test.js b/packages/material-ui/src/TableRow/TableRow.test.js
index b2c9198c341051..afe3d271ede37c 100644
--- a/packages/material-ui/src/TableRow/TableRow.test.js
+++ b/packages/material-ui/src/TableRow/TableRow.test.js
@@ -1,12 +1,14 @@
 import * as React from 'react';
-import { assert } from 'chai';
+import { assert, expect } from 'chai';
 import { createMount, getClasses } from '@material-ui/core/test-utils';
 import describeConformance from '../test-utils/describeConformance';
+import { createClientRender } from 'test/utils/createClientRender';
 import TableRow from './TableRow';
 
 describe('<TableRow />', () => {
   let mount;
   let classes;
+  const render = createClientRender();
   function mountInTable(node) {
     const wrapper = mount(
       <table>
@@ -38,4 +40,16 @@ describe('<TableRow />', () => {
     const wrapper = mountInTable(<TableRow>{children}</TableRow>);
     assert.strictEqual(wrapper.contains(children), true);
   });
+
+  describe('prop: component', () => {
+    it('can render a different component', () => {
+      const { container } = render(<TableRow component="div" />);
+      expect(container.firstChild).to.have.property('nodeName', 'DIV');
+    });
+
+    it('sets role="rowgroup"', () => {
+      const { container } = render(<TableRow component="div" />);
+      expect(container.firstChild).to.have.attribute('role', 'row');
+    });
+  });
 });
diff --git a/packages/material-ui/test/integration/TableCell.test.js b/packages/material-ui/test/integration/TableCell.test.js
index 301c60bc18d33e..1ac1e702e67c60 100644
--- a/packages/material-ui/test/integration/TableCell.test.js
+++ b/packages/material-ui/test/integration/TableCell.test.js
@@ -1,13 +1,16 @@
 import * as React from 'react';
-import { assert } from 'chai';
+import { assert, expect } from 'chai';
 import { createMount, findOutermostIntrinsic, getClasses } from '@material-ui/core/test-utils';
+import { createClientRender } from 'test/utils/createClientRender';
 import TableCell from '@material-ui/core/TableCell';
 import TableFooter from '@material-ui/core/TableFooter';
 import TableHead from '@material-ui/core/TableHead';
+import TableBody from '@material-ui/core/TableBody';
 
 describe('<TableRow> integration', () => {
   let classes;
   let mount;
+  const render = createClientRender();
   function mountInTable(node, Variant) {
     const wrapper = mount(
       <table>
@@ -76,4 +79,31 @@ describe('<TableRow> integration', () => {
     const wrapper = mountInTable(<TableCell variant="footer" />, TableHead);
     assert.strictEqual(wrapper.find('th').hasClass(classes.footer), true);
   });
+
+  it('sets role="columnheader" when "component" prop is set and used in the context of table head', () => {
+    const { getByTestId } = render(
+      <TableHead component="div">
+        <TableCell component="div" data-testid="cell" />,
+      </TableHead>,
+    );
+    expect(getByTestId('cell')).to.have.attribute('role', 'columnheader');
+  });
+
+  it('sets role="cell" when "component" prop is set and used in the context of table body ', () => {
+    const { getByTestId } = render(
+      <TableBody component="div">
+        <TableCell component="div" data-testid="cell" />,
+      </TableBody>,
+    );
+    expect(getByTestId('cell')).to.have.attribute('role', 'cell');
+  });
+
+  it('sets role="cell" when "component" prop is set and used in the context of table footer ', () => {
+    const { getByTestId } = render(
+      <TableFooter component="div">
+        <TableCell component="div" data-testid="cell" />,
+      </TableFooter>,
+    );
+    expect(getByTestId('cell')).to.have.attribute('role', 'cell');
+  });
 });