Merge pull request #18515 from nwpappas/status-a11y-fix

Fix Status a11y concerns

.changeset/four-humans-applaud.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': patch
+---
+
+Updates the Status component to fix use of color and voice over accessibility issues.

packages/core-components/src/components/Status/Status.test.tsx

@@ -28,42 +28,52 @@ import {
 
 describe('<StatusOK />', () => {
   it('renders without exploding', async () => {
-    const { getByLabelText } = await renderInTestApp(<StatusOK />);
-    expect(getByLabelText('Status ok')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(<StatusOK>OK</StatusOK>);
+    expect(getByText('OK')).toBeInTheDocument();
   });
 });
 
 describe('<StatusWarning />', () => {
   it('renders without exploding', async () => {
-    const { getByLabelText } = await renderInTestApp(<StatusWarning />);
-    expect(getByLabelText('Status warning')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(
+      <StatusWarning>Warning</StatusWarning>,
+    );
+    expect(getByText('Warning')).toBeInTheDocument();
   });
 });
 
 describe('<StatusError />', () => {
   it('renders without exploding', async () => {
-    const { getByLabelText } = await renderInTestApp(<StatusError />);
-    expect(getByLabelText('Status error')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(
+      <StatusError>Error</StatusError>,
+    );
+    expect(getByText('Error')).toBeInTheDocument();
   });
 });
 
 describe('<StatusPending />', () => {
   it('renders without exploding', async () => {
-    const { getByLabelText } = await renderInTestApp(<StatusPending />);
-    expect(getByLabelText('Status pending')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(
+      <StatusPending>Pending</StatusPending>,
+    );
+    expect(getByText('Pending')).toBeInTheDocument();
   });
 });
 
 describe('<StatusRunning />', () => {
   it('renders without exploding', async () => {
-    const { getByLabelText } = await renderInTestApp(<StatusRunning />);
-    expect(getByLabelText('Status running')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(
+      <StatusRunning>Running</StatusRunning>,
+    );
+    expect(getByText('Running')).toBeInTheDocument();
   });
 });
 
 describe('<StatusAborted />', () => {
   it('renders without exploding', async () => {
-    const { getByLabelText } = await renderInTestApp(<StatusAborted />);
-    expect(getByLabelText('Status aborted')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(
+      <StatusAborted>Aborted</StatusAborted>,
+    );
+    expect(getByText('Aborted')).toBeInTheDocument();
   });
 });

packages/core-components/src/components/Status/Status.tsx

@@ -19,6 +19,13 @@ import Typography from '@material-ui/core/Typography';
 import classNames from 'classnames';
 import React, { PropsWithChildren } from 'react';
 
+import CheckIcon from '@material-ui/icons/Check';
+import WarningIcon from '@material-ui/icons/Warning';
+import ErrorIcon from '@material-ui/icons/Error';
+import HighlightOffIcon from '@material-ui/icons/HighlightOff';
+import ScheduleIcon from '@material-ui/icons/Schedule';
+import DirectionsRunIcon from '@material-ui/icons/DirectionsRun';
+
 export type StatusClassKey =
   | 'status'
   | 'ok'
@@ -31,44 +38,43 @@ export type StatusClassKey =
 const useStyles = makeStyles<BackstageTheme>(
   theme => ({
     status: {
+      alignItems: 'center',
+      display: 'flex',
+      flexFlow: 'row wrap',
       fontWeight: theme.typography.fontWeightMedium,
-      '&::before': {
-        width: '0.7em',
-        height: '0.7em',
-        display: 'inline-block',
+
+      '& > svg': {
         marginRight: theme.spacing(1),
-        borderRadius: '50%',
-        content: '""',
       },
     },
     ok: {
-      '&::before': {
-        backgroundColor: theme.palette.status.ok,
+      '& > svg': {
+        fill: theme.palette.status.ok,
       },
     },
     warning: {
-      '&::before': {
-        backgroundColor: theme.palette.status.warning,
+      '& > svg': {
+        fill: theme.palette.status.warning,
       },
     },
     error: {
-      '&::before': {
-        backgroundColor: theme.palette.status.error,
+      '& > svg': {
+        fill: theme.palette.status.error,
       },
     },
     pending: {
-      '&::before': {
-        backgroundColor: theme.palette.status.pending,
+      '& > svg': {
+        fill: theme.palette.status.pending,
       },
     },
     running: {
-      '&::before': {
-        backgroundColor: theme.palette.status.running,
+      '& > svg': {
+        fill: theme.palette.status.running,
       },
     },
     aborted: {
-      '&::before': {
-        backgroundColor: theme.palette.status.aborted,
+      '& > svg': {
+        fill: theme.palette.status.aborted,
       },
     },
   }),
@@ -77,78 +83,85 @@ const useStyles = makeStyles<BackstageTheme>(
 
 export function StatusOK(props: PropsWithChildren<{}>) {
   const classes = useStyles(props);
+
   return (
-    <Typography
-      component="span"
+    <div
       className={classNames(classes.status, classes.ok)}
-      aria-label="Status ok"
-      aria-hidden="true"
-      {...props}
-    />
+      aria-label={!props.children ? 'Ok' : undefined}
+      role={!props.children ? 'img' : undefined}
+    >
+      <CheckIcon style={{ fontSize: '1rem' }} />
+      <Typography component="span" {...props} />
+    </div>
   );
 }
 
 export function StatusWarning(props: PropsWithChildren<{}>) {
   const classes = useStyles(props);
   return (
-    <Typography
-      component="span"
+    <div
       className={classNames(classes.status, classes.warning)}
-      aria-label="Status warning"
-      aria-hidden="true"
-      {...props}
-    />
+      aria-label={!props.children ? 'Warning' : undefined}
+      role={!props.children ? 'img' : undefined}
+    >
+      <WarningIcon style={{ fontSize: '1rem' }} />
+      <Typography component="span" {...props} />
+    </div>
   );
 }
 
 export function StatusError(props: PropsWithChildren<{}>) {
   const classes = useStyles(props);
   return (
-    <Typography
-      component="span"
+    <div
       className={classNames(classes.status, classes.error)}
-      aria-label="Status error"
-      aria-hidden="true"
-      {...props}
-    />
+      aria-label={!props.children ? 'Error' : undefined}
+      role={!props.children ? 'img' : undefined}
+    >
+      <ErrorIcon style={{ fontSize: '1rem' }} />
+      <Typography component="span" {...props} />
+    </div>
   );
 }
 
 export function StatusPending(props: PropsWithChildren<{}>) {
   const classes = useStyles(props);
   return (
-    <Typography
-      component="span"
+    <div
       className={classNames(classes.status, classes.pending)}
-      aria-label="Status pending"
-      aria-hidden="true"
-      {...props}
-    />
+      aria-label={!props.children ? 'Pending' : undefined}
+      role={!props.children ? 'img' : undefined}
+    >
+      <ScheduleIcon style={{ fontSize: '1rem' }} />
+      <Typography component="span" {...props} />
+    </div>
   );
 }
 
 export function StatusRunning(props: PropsWithChildren<{}>) {
   const classes = useStyles(props);
   return (
-    <Typography
-      component="span"
+    <div
       className={classNames(classes.status, classes.running)}
-      aria-label="Status running"
-      aria-hidden="true"
-      {...props}
-    />
+      aria-label={!props.children ? 'Running' : undefined}
+      role={!props.children ? 'img' : undefined}
+    >
+      <DirectionsRunIcon style={{ fontSize: '1rem' }} />
+      <Typography component="span" {...props} />
+    </div>
   );
 }
 
 export function StatusAborted(props: PropsWithChildren<{}>) {
   const classes = useStyles(props);
   return (
-    <Typography
-      component="span"
+    <div
       className={classNames(classes.status, classes.aborted)}
-      aria-label="Status aborted"
-      aria-hidden="true"
-      {...props}
-    />
+      aria-label={!props.children ? 'Aborted' : undefined}
+      role={!props.children ? 'img' : undefined}
+    >
+      <HighlightOffIcon style={{ fontSize: '1rem' }} />
+      <Typography component="span" {...props} />
+    </div>
   );
 }

plugins/xcmetrics/src/components/StatusIcon/StatusIcon.test.tsx

@@ -24,19 +24,19 @@ describe('StatusIcon', () => {
     let rendered = await renderInTestApp(
       <StatusIcon buildStatus="succeeded" />,
     );
-    expect(rendered.getByLabelText('Status ok')).toBeInTheDocument();
+    expect(rendered.getByLabelText('Ok')).toBeInTheDocument();
 
     rendered = await renderInTestApp(<StatusIcon buildStatus="failed" />);
-    expect(rendered.getByLabelText('Status error')).toBeInTheDocument();
+    expect(rendered.getByLabelText('Error')).toBeInTheDocument();
 
     rendered = await renderInTestApp(<StatusIcon buildStatus="stopped" />);
-    expect(rendered.getByLabelText('Status warning')).toBeInTheDocument();
+    expect(rendered.getByLabelText('Warning')).toBeInTheDocument();
   });
 
   it('should render invalid statuses', async () => {
     const rendered = await renderInTestApp(
       <StatusIcon buildStatus={'invalid' as BuildStatus} />,
     );
-    expect(rendered.getByLabelText('Status aborted')).toBeInTheDocument();
+    expect(rendered.getByLabelText('Aborted')).toBeInTheDocument();
   });
 });