TableCell should not add role="cell" if a th component is passed

[tedwardmah]

The docs for a Simple Table say:

For accessibility, the first column is set to be a <th> element, with a scope of "col". This enables screen readers to identify a cell's value by it's row and column name.

The screen reader behavior referred to in the docs is no longer working in the Simple Table example, at least not using VoiceOver.

I believe this is because the cells are now being given a role="cell".

#20475 introduced the concept of a default role when passing a component to the TableCell via props. It definitely makes sense to add a role for accessibility when passing custom components, but probably not in cases where the component we are passing has native roles already (like a "th" tag)?

If that's not possible, as a user I'd at least like to have some sort of override, by passing a role directly or some other prop flag perhaps.

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The following:

<TableCell component="th" scope="row">Cell content</TableCell>

Renders:

<th class="..." role="cell" scope="row">Cell content</th>

Expected Behavior 🤔

Should render:

<th class="..." scope="row">Cell content</th>

Steps to Reproduce 🕹

Simple Table documentation can be used to verify the issue

Context 🔦

I'm using a nearly identical approach in my application, and after upgrading to v4.10.1 I have failing unit tests because cells that used to be recognized by their "rowheader" role are no longer recognized. Screen reader behavior that reads row titles when navigating the table is also broken in my application.

Your Environment 🌎

Tech	Version
Material-UI	v4.10.1
React	v16.13.0
Browser	Chrome Version 83.0.4103.61 (Official Build) (64-bit)

[eps1lon]

I agree that we just shouldn't add the role and let users fix this on their own. I would argue this for any component usage since we can't reliably reason about it.

Almost all cases I see are invalid anyway (abuses styles without semantics, inaccessible regardless etc).

The original motivation in #20431 can be easily fixed with <TableCell component="div" role="cell" /> which is better anyway. It makes it clear that, despite not using semantic HTML you still want the accessible semantics. I would expect that <TableCell component="div" /> wants to work around cell semantics.

[tchmnn]

So this issue is now about removing the behaviour to set the role?

Anyway, I've looked into it really quickly and came up with this

diff --git a/packages/material-ui/src/TableCell/TableCell.js b/packages/material-ui/src/TableCell/TableCell.js
index d2d044720..6178f9b71 100644
--- a/packages/material-ui/src/TableCell/TableCell.js
+++ b/packages/material-ui/src/TableCell/TableCell.js
@@ -123,19 +123,27 @@ const TableCell = React.forwardRef(function TableCell(props, ref) {
   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 = isHeadCell ? 'th' : 'td';
   }
-
+  
   let scope = scopeProp;
   if (!scope && isHeadCell) {
     scope = 'col';
   }
+  
+  let role;
+  if (isHeadCell) {
+    role = 'columnheader';
+  } else if (scope === 'row') {
+    role = 'row';
+  } else {
+    role = 'cell';
+  }
+
   const padding = paddingProp || (table && table.padding ? table.padding : 'default');
   const size = sizeProp || (table && table.size ? table.size : 'medium');
   const variant = variantProp || (tablelvl2 && tablelvl2.variant);

[eps1lon]

I'd just revert #20431 and add some regression test that illustrate why the approach wasn't viable e.g. <TableCell component={CustomeTableHead} scope="row" /> renders a role colgroup.

[nharraud]

Hi all,

I just got bitten by this issue too. I can't use rowheader role with testing-library because of the forced cell role.

@tchmnn I might be missing something but it looks like your patch generates <th role="row" scope="row"> when <TableCell component="th" scope="row"> is used. As @tedwardmah said in his original post, the expected role should be rowheader, but it is unnecessary to set this role for th elements as it is implicit.