Material-ui: [Breadcrumb] Visual separators included in list item count for screen reader

[ChantalDesrochers]

The accessibility section at the bottom of this link: https://material-ui.com/components/breadcrumbs/ indicates the use of 'aria-hidden' to prevent screen reader announcement of the visual separators between links.

When using ChromeVox, the separators are not read out, however, the separators are included in the item count of the ordered list, providing misleading information to the user.

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

Current Behavior 😯

Sample breadcrumbs:

Home / About Us / Contact

Screen reader reads:

tab -> 'ordered list with 5 items, Home link list item'
tab -> 'About Us, link list item'
tab -> 'Contact, link list item'

• includes separators in list item count

Expected Behavior 🤔

Sample breadcrumbs:

Home / About Us / Contact

Screen reader reads:

tab -> 'ordered list with 3 items, Home link list item'
tab -> 'About Us, link list item'
tab -> 'Contact, link list item'

• should not include separators in list item count
• screen reader should read out that it is an ordered list with 3 items

Steps to Reproduce 🕹

Visit https://material-ui.com/components/breadcrumbs/
Steps:

1. Turn on screen reader
2. Tab to breadcrumb demo
3. Listen to screen reader read out number of items in list

Environment 🌎

| Material-UI/core | 4.9.8 |
| React | 16.8.6 |
| Chrome | 83.0.4103.116 |
| ChromeVox Classic Extension | 53.0.2784.6 |

[mbrookes]

Resources:

• https://www.w3.org/TR/wai-aria-practices-1.1/examples/breadcrumb/index.html
• https://www.aditus.io/patterns/breadcrumbs/
• https://scottaohara.github.io/a11y_breadcrumbs/
• https://a11y-style-guide.com/style-guide/section-navigation.html#kssref-navigation-breadcrumbs

[oliviertassinari]

@ChantalDesrochers Thanks for raising. It seems to behave correctly on VoiceOver, it announces 3 elements, but I can reproduce the same with ChromeVox. Should we fill an issue to ChromeVox?

The only alternative I can think of is to change the API to:

<Breadcrumbs>
  <Breadcrumb>
    <Link>A</Link>
  </Breadcrumb>
  <Breadcrumb>
    <Link>B</Link>
  </Breadcrumb>
  <Breadcrumb>
    <Link>C</Link>
  </Breadcrumb>
</Breadcrumbs>

Not a huge fan.

[mbrookes]

@oliviertassinari Why does it require a change of API?

[oliviertassinari]

From what I understand there are only 3 possible outcomes:

1. It's a bug in ChromeVox only, we open an issue and let it sleep.
2. We change the API to avoid extra <li> element.
3. We use the cloneElement API to apply 2. It seems too brittle.

Do you see something else or different?

[mbrookes]

--- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js
+++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js
@@ -8,6 +8,9 @@ import ButtonBase from '../ButtonBase';
 const styles = (theme) => ({
   root: {
     display: 'flex',
+  },
+  buttonBase: {
+    display: 'flex',
     marginLeft: theme.spacing(0.5),
     marginRight: theme.spacing(0.5),
     backgroundColor: theme.palette.grey[100],
@@ -26,18 +29,30 @@ const styles = (theme) => ({
     width: 24,
     height: 16,
   },
+  /* Styles applied to the separator element. */
+  separator: {
+    display: 'flex',
+    userSelect: 'none',
+    marginLeft: 8,
+    marginRight: 8,
+  },
 });

 /**
  * @ignore - internal component.
  */
 function BreadcrumbCollapsed(props) {
-  const { classes, ...other } = props;
+  const { classes, separator, ...other } = props;

   return (
-    <ButtonBase component="li" className={classes.root} focusRipple {...other}>
-      <MoreHorizIcon className={classes.icon} />
-    </ButtonBase>
+    <li className={classes.root} {...other}>
+      <ButtonBase className={classes.buttonBase} focusRipple>
+        <MoreHorizIcon className={classes.icon} />
+      </ButtonBase>
+      <span className={classes.separator} aria-hidden="true">
+        {separator}
+      </span>
+    </li>
   );
 }

@@ -46,6 +61,10 @@ BreadcrumbCollapsed.propTypes = {
    * @ignore
    */
   classes: PropTypes.object.isRequired,
+  /**
+   * Separator node.
+   */
+  separator: PropTypes.node,
 };

 export default withStyles(styles, { name: 'PrivateBreadcrumbCollapsed' })(BreadcrumbCollapsed);
diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
index 41189fb89..a150b8d6b 100644
--- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
+++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
@@ -19,7 +19,9 @@ export const styles = {
     listStyle: 'none',
   },
   /* Styles applied to the li element. */
-  li: {},
+  li: {
+    display: 'flex',
+  },
   /* Styles applied to the separator element. */
   separator: {
     display: 'flex',
@@ -29,23 +31,6 @@ export const styles = {
   },
 };

-function insertSeparators(items, className, separator) {
-  return items.reduce((acc, current, index) => {
-    if (index < items.length - 1) {
-      acc = acc.concat(
-        current,
-        <li aria-hidden key={`separator-${index}`} className={className}>
-          {separator}
-        </li>,
-      );
-    } else {
-      acc.push(current);
-    }
-
-    return acc;
-  }, []);
-}
-
 const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {
   const {
     children,
@@ -90,7 +75,12 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {

     return [
       ...allItems.slice(0, itemsBeforeCollapse),
-      <BreadcrumbCollapsed aria-label={expandText} key="ellipsis" onClick={handleClickExpand} />,
+      <BreadcrumbCollapsed
+        separator={separator}
+        aria-label={expandText}
+        key="ellipsis"
+        onClick={handleClickExpand}
+      />,
       ...allItems.slice(allItems.length - itemsAfterCollapse, allItems.length),
     ];
   };
@@ -113,6 +103,11 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {
     .map((child, index) => (
       <li className={classes.li} key={`child-${index}`}>
         {child}
+        {index < children.length - 1 && (
+          <span className={classes.separator} aria-hidden="true">
+            {separator}
+          </span>
+        )}
       </li>
     ));

@@ -125,13 +120,9 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {
       {...other}
     >
       <ol className={classes.ol}>
-        {insertSeparators(
-          expanded || (maxItems && allItems.length <= maxItems)
-            ? allItems
-            : renderItemsBeforeAndAfter(allItems),
-          classes.separator,
-          separator,
-        )}
+        {expanded || (maxItems && allItems.length <= maxItems)
+          ? allItems
+          : renderItemsBeforeAndAfter(allItems)}
       </ol>
     </Typography>
   );

[oliviertassinari]

Right, interesting, this sounds like a viable option too :)

[oliviertassinari]

Opened https://bugs.chromium.org/p/chromium/issues/detail?id=1101725.

[oliviertassinari]

The related issue on Chrome side was closed:

ChromeVox Classic extension is no longer supported