fix: use aria-hidden to hide icons from screen readers

Instead of the presentation role. After upgrading axe-core, we're now getting failures for presentation not being a valid role for <svg> elements. I'm not 100% sure that's the case. But we can see that axe-core has an allowlist for what elements can have that role [1], and svg is not on it. There doesn't seem to be documentation anywhere that it's explicitly not allowed. But this change has the same effect, and plays nicely with axe. Also, MDN lists some interesting differences between role="presentation" and aria-hidden [2]. aria-hidden is for literally hiding something from assistive technology, while role="presentation" only strips an element of any semantics. Hiding it in this case seems reasonable and what we actually intend. 1. https://github.com/dequelabs/axe-core/blob/2777dbc20c028d88a906e49a3c d7032b482f0988/lib/commons/aria/index.js#L1462-L1488 2. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_ Techniques/Using_the_aria-hidden_attribute
chanzuckerberg · Aug 2, 2021 · b656013 · b656013
1 parent 7260aa5
commit b656013
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 44 deletions.
diff --git a/packages/components/src/Button/__snapshots__/button.spec.tsx.snap b/packages/components/src/Button/__snapshots__/button.spec.tsx.snap
@@ -60,12 +60,14 @@ exports[`<Button /> linkWithIcon story renders snapshot 1`] = `
   
   Link with icon 
   <svg
-    role="presentation"
-    style="height: 24px;"
-    viewBox="0 0 20 20"
+    aria-hidden="true"
+    class="svgIcon"
+    fill="currentColor"
+    viewBox="0 0 24 24"
+    xmlns="http://www.w3.org/2000/svg"
   >
     <path
-      d="M9.719,17.073l-6.562-6.51c-0.27-0.268-0.504-0.567-0.696-0.888C1.385,7.89,1.67,5.613,3.155,4.14c0.864-0.856,2.012-1.329,3.233-1.329c1.924,0,3.115,1.12,3.612,1.752c0.499-0.634,1.689-1.752,3.612-1.752c1.221,0,2.369,0.472,3.233,1.329c1.484,1.473,1.771,3.75,0.693,5.537c-0.19,0.32-0.425,0.618-0.695,0.887l-6.562,6.51C10.125,17.229,9.875,17.229,9.719,17.073 M6.388,3.61C5.379,3.61,4.431,4,3.717,4.707C2.495,5.92,2.259,7.794,3.145,9.265c0.158,0.265,0.351,0.51,0.574,0.731L10,16.228l6.281-6.232c0.224-0.221,0.416-0.466,0.573-0.729c0.887-1.472,0.651-3.346-0.571-4.56C15.57,4,14.621,3.61,13.612,3.61c-1.43,0-2.639,0.786-3.268,1.863c-0.154,0.264-0.536,0.264-0.69,0C9.029,4.397,7.82,3.61,6.388,3.61"
+      d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm-2 15l-5-5 1.41-1.41L10 14.17l7.59-7.59L19 8l-9 9z"
     />
   </svg>
 </button>
@@ -89,12 +91,14 @@ exports[`<Button /> outlineWithIcon story renders snapshot 1`] = `
   
   Outline with icon 
   <svg
-    role="presentation"
-    style="height: 24px;"
-    viewBox="0 0 20 20"
+    aria-hidden="true"
+    class="svgIcon"
+    fill="currentColor"
+    viewBox="0 0 24 24"
+    xmlns="http://www.w3.org/2000/svg"
   >
     <path
-      d="M9.719,17.073l-6.562-6.51c-0.27-0.268-0.504-0.567-0.696-0.888C1.385,7.89,1.67,5.613,3.155,4.14c0.864-0.856,2.012-1.329,3.233-1.329c1.924,0,3.115,1.12,3.612,1.752c0.499-0.634,1.689-1.752,3.612-1.752c1.221,0,2.369,0.472,3.233,1.329c1.484,1.473,1.771,3.75,0.693,5.537c-0.19,0.32-0.425,0.618-0.695,0.887l-6.562,6.51C10.125,17.229,9.875,17.229,9.719,17.073 M6.388,3.61C5.379,3.61,4.431,4,3.717,4.707C2.495,5.92,2.259,7.794,3.145,9.265c0.158,0.265,0.351,0.51,0.574,0.731L10,16.228l6.281-6.232c0.224-0.221,0.416-0.466,0.573-0.729c0.887-1.472,0.651-3.346-0.571-4.56C15.57,4,14.621,3.61,13.612,3.61c-1.43,0-2.639,0.786-3.268,1.863c-0.154,0.264-0.536,0.264-0.69,0C9.029,4.397,7.82,3.61,6.388,3.61"
+      d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm-2 15l-5-5 1.41-1.41L10 14.17l7.59-7.59L19 8l-9 9z"
     />
   </svg>
 </button>

diff --git a/packages/components/src/Button/button.stories.tsx b/packages/components/src/Button/button.stories.tsx
@@ -1,4 +1,5 @@
 import Button from "./button";
+import { CheckCircle } from "../SvgIcon/Icons";
 import React from "react";
 import { Story } from "@storybook/react/types-6-0";
 import Text from "../Text";
@@ -19,12 +20,6 @@ type Args = React.ComponentProps<typeof Button>;
 
 const Template: Story<Args> = (args) => <Button {...args} />;
 
-const heartIcon = (
-  <svg style={{ height: "24px" }} viewBox="0 0 20 20" role="presentation">
-    <path d="M9.719,17.073l-6.562-6.51c-0.27-0.268-0.504-0.567-0.696-0.888C1.385,7.89,1.67,5.613,3.155,4.14c0.864-0.856,2.012-1.329,3.233-1.329c1.924,0,3.115,1.12,3.612,1.752c0.499-0.634,1.689-1.752,3.612-1.752c1.221,0,2.369,0.472,3.233,1.329c1.484,1.473,1.771,3.75,0.693,5.537c-0.19,0.32-0.425,0.618-0.695,0.887l-6.562,6.51C10.125,17.229,9.875,17.229,9.719,17.073 M6.388,3.61C5.379,3.61,4.431,4,3.717,4.707C2.495,5.92,2.259,7.794,3.145,9.265c0.158,0.265,0.351,0.51,0.574,0.731L10,16.228l6.281-6.232c0.224-0.221,0.416-0.466,0.573-0.729c0.887-1.472,0.651-3.346-0.571-4.56C15.57,4,14.621,3.61,13.612,3.61c-1.43,0-2.639,0.786-3.268,1.863c-0.154,0.264-0.536,0.264-0.69,0C9.029,4.397,7.82,3.61,6.388,3.61"></path>
-  </svg>
-);
-
 export const flat = Template.bind(null);
 flat.args = {
   children: "Flat Button",
@@ -79,27 +74,25 @@ withDataTestId.args = {
 
 export const linkWithIcon = Template.bind(null);
 linkWithIcon.args = {
-  children: <>Link with icon {heartIcon}</>,
+  children: (
+    <>
+      Link with icon <CheckCircle role="presentation" />
+    </>
+  ),
   color: "success",
   variant: "link",
 };
-linkWithIcon.parameters = {
-  axe: {
-    disabledRules: ["aria-allowed-role"],
-  },
-};
 
 export const outlineWithIcon = Template.bind(null);
 outlineWithIcon.args = {
-  children: <>Outline with icon {heartIcon}</>,
+  children: (
+    <>
+      Outline with icon <CheckCircle role="presentation" />
+    </>
+  ),
   color: "warning",
   variant: "outline",
 };
-outlineWithIcon.parameters = {
-  axe: {
-    disabledRules: ["aria-allowed-role"],
-  },
-};
 
 export const withFakeClassName = Template.bind(null);
 withFakeClassName.args = {

diff --git a/packages/components/src/SvgIcon/SvgIcon.stories.tsx b/packages/components/src/SvgIcon/SvgIcon.stories.tsx
@@ -35,12 +35,6 @@ export default {
       },
     },
   },
-
-  parameters: {
-    axe: {
-      disabledRules: ["aria-allowed-role"],
-    },
-  },
 };
 
 type Args = React.ComponentProps<typeof SvgIcon> & {
@@ -84,7 +78,7 @@ FullScreen.args = {
 
 FullScreen.parameters = {
   axe: {
-    disabledRules: ["aria-allowed-role", "scrollable-region-focusable"],
+    disabledRules: ["scrollable-region-focusable"],
   },
 };
 
@@ -150,7 +144,7 @@ AllIcons.parameters = {
     skip: true,
   },
   axe: {
-    disabledRules: ["aria-allowed-role", "scrollable-region-focusable"],
+    disabledRules: ["scrollable-region-focusable"],
   },
   controls: {
     hideNoControlsWarning: true,

diff --git a/packages/components/src/SvgIcon/SvgIcon.tsx b/packages/components/src/SvgIcon/SvgIcon.tsx
@@ -65,7 +65,6 @@ function SvgIcon(props: SvgIconProps) {
     children,
     className,
     color = "currentColor",
-    role,
     size,
     viewBox = "0 0 24 24",
   } = props;
@@ -78,7 +77,6 @@ function SvgIcon(props: SvgIconProps) {
     className: clsx(className, styles.svgIcon, block && styles.displayBlock),
     fill: color,
     height: size,
-    role,
     /**
      * height/width html properties are overriden by the defaults applied
      * to svg css class
@@ -91,13 +89,17 @@ function SvgIcon(props: SvgIconProps) {
 
   if (props.role === "img") {
     return (
-      <svg {...svgCommonProps}>
+      <svg {...svgCommonProps} role="img">
         <title>{props.title}</title>
         {children}
       </svg>
     );
   } else {
-    return <svg {...svgCommonProps}>{children}</svg>;
+    return (
+      <svg {...svgCommonProps} aria-hidden>
+        {children}
+      </svg>
+    );
   }
 }
 

diff --git a/packages/components/src/SvgIcon/__snapshots__/SvgIcon.spec.tsx.snap b/packages/components/src/SvgIcon/__snapshots__/SvgIcon.spec.tsx.snap
@@ -2,9 +2,9 @@
 
 exports[`<SvgIcon /> CustomColor story renders snapshot 1`] = `
 <svg
+  aria-hidden="true"
   class="svgIcon"
   fill="#928CFF"
-  role="presentation"
   viewBox="0 0 24 24"
   xmlns="http://www.w3.org/2000/svg"
 >
@@ -16,9 +16,9 @@ exports[`<SvgIcon /> CustomColor story renders snapshot 1`] = `
 
 exports[`<SvgIcon /> CustomViewBox story renders snapshot 1`] = `
 <svg
+  aria-hidden="true"
   class="svgIcon"
   fill="currentColor"
-  role="presentation"
   viewBox="4 4 16 16"
   xmlns="http://www.w3.org/2000/svg"
 >
@@ -30,10 +30,10 @@ exports[`<SvgIcon /> CustomViewBox story renders snapshot 1`] = `
 
 exports[`<SvgIcon /> FullScreen story renders snapshot 1`] = `
 <svg
+  aria-hidden="true"
   class="svgIcon"
   fill="currentColor"
   height="min(100%, 100vh)"
-  role="presentation"
   style="--svg-icon-size: min(100%, 100vh);"
   viewBox="0 0 24 24"
   width="min(100%, 100vh)"
@@ -97,10 +97,10 @@ exports[`<SvgIcon /> InText story renders snapshot 1`] = `
 
 exports[`<SvgIcon /> Large story renders snapshot 1`] = `
 <svg
+  aria-hidden="true"
   class="svgIcon"
   fill="currentColor"
   height="4em"
-  role="presentation"
   style="--svg-icon-size: 4em;"
   viewBox="0 0 24 24"
   width="4em"
@@ -114,10 +114,10 @@ exports[`<SvgIcon /> Large story renders snapshot 1`] = `
 
 exports[`<SvgIcon /> Medium story renders snapshot 1`] = `
 <svg
+  aria-hidden="true"
   class="svgIcon"
   fill="currentColor"
   height="2em"
-  role="presentation"
   style="--svg-icon-size: 2em;"
   viewBox="0 0 24 24"
   width="2em"
@@ -131,10 +131,10 @@ exports[`<SvgIcon /> Medium story renders snapshot 1`] = `
 
 exports[`<SvgIcon /> Small story renders snapshot 1`] = `
 <svg
+  aria-hidden="true"
   class="svgIcon"
   fill="currentColor"
   height="1em"
-  role="presentation"
   style="--svg-icon-size: 1em;"
   viewBox="0 0 24 24"
   width="1em"