From 1a1e475696b98444166e6fd9f3960b5b07883be3 Mon Sep 17 00:00:00 2001
From: Nicholas Pappas <nwpappas@gmail.com>
Date: Fri, 30 Jun 2023 20:25:33 -0700
Subject: [PATCH 1/3] Fix Status a11y concerns

Signed-off-by: Nicholas Pappas <nwpappas@gmail.com>
---
 .../src/components/Status/Status.test.tsx     |  34 +++--
 .../src/components/Status/Status.tsx          | 121 ++++++++++--------
 2 files changed, 89 insertions(+), 66 deletions(-)

diff --git a/packages/core-components/src/components/Status/Status.test.tsx b/packages/core-components/src/components/Status/Status.test.tsx
index c1e1d42af2..dc80acecc9 100644
--- a/packages/core-components/src/components/Status/Status.test.tsx
+++ b/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();
   });
 });
diff --git a/packages/core-components/src/components/Status/Status.tsx b/packages/core-components/src/components/Status/Status.tsx
index 364193e383..f3bdf4e0f2 100644
--- a/packages/core-components/src/components/Status/Status.tsx
+++ b/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 CheckCircleIcon from '@material-ui/icons/CheckCircle';
+import WarningIcon from '@material-ui/icons/Warning';
+import ErrorIcon from '@material-ui/icons/Error';
+import HighlightOffIcon from '@material-ui/icons/HighlightOff';
+import WatchLaterIcon from '@material-ui/icons/WatchLater';
+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}
+    >
+      <CheckCircleIcon 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}
+    >
+      <WatchLaterIcon 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>
   );
 }

From d6d1571b0cfce9653d74941618f1bf761850a0a5 Mon Sep 17 00:00:00 2001
From: Nicholas Pappas <nwpappas@gmail.com>
Date: Fri, 30 Jun 2023 20:57:02 -0700
Subject: [PATCH 2/3] Adding changeset

Signed-off-by: Nicholas Pappas <nwpappas@gmail.com>
---
 .changeset/four-humans-applaud.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/four-humans-applaud.md

diff --git a/.changeset/four-humans-applaud.md b/.changeset/four-humans-applaud.md
new file mode 100644
index 0000000000..50ab681de9
--- /dev/null
+++ b/.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.

From edc3b38a70f6e59d8f11b4b2a05bc2d615069048 Mon Sep 17 00:00:00 2001
From: Nicholas Pappas <nwpappas@gmail.com>
Date: Fri, 7 Jul 2023 15:44:12 -0700
Subject: [PATCH 3/3] Icon updates

Updated a few icons to (maybe) better ones.

Signed-off-by: Nicholas Pappas <nwpappas@gmail.com>
---
 packages/core-components/src/components/Status/Status.tsx | 8 ++++----
 .../src/components/StatusIcon/StatusIcon.test.tsx         | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/core-components/src/components/Status/Status.tsx b/packages/core-components/src/components/Status/Status.tsx
index f3bdf4e0f2..ffe01cdb1b 100644
--- a/packages/core-components/src/components/Status/Status.tsx
+++ b/packages/core-components/src/components/Status/Status.tsx
@@ -19,11 +19,11 @@ import Typography from '@material-ui/core/Typography';
 import classNames from 'classnames';
 import React, { PropsWithChildren } from 'react';
 
-import CheckCircleIcon from '@material-ui/icons/CheckCircle';
+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 WatchLaterIcon from '@material-ui/icons/WatchLater';
+import ScheduleIcon from '@material-ui/icons/Schedule';
 import DirectionsRunIcon from '@material-ui/icons/DirectionsRun';
 
 export type StatusClassKey =
@@ -90,7 +90,7 @@ export function StatusOK(props: PropsWithChildren<{}>) {
       aria-label={!props.children ? 'Ok' : undefined}
       role={!props.children ? 'img' : undefined}
     >
-      <CheckCircleIcon style={{ fontSize: '1rem' }} />
+      <CheckIcon style={{ fontSize: '1rem' }} />
       <Typography component="span" {...props} />
     </div>
   );
@@ -132,7 +132,7 @@ export function StatusPending(props: PropsWithChildren<{}>) {
       aria-label={!props.children ? 'Pending' : undefined}
       role={!props.children ? 'img' : undefined}
     >
-      <WatchLaterIcon style={{ fontSize: '1rem' }} />
+      <ScheduleIcon style={{ fontSize: '1rem' }} />
       <Typography component="span" {...props} />
     </div>
   );
diff --git a/plugins/xcmetrics/src/components/StatusIcon/StatusIcon.test.tsx b/plugins/xcmetrics/src/components/StatusIcon/StatusIcon.test.tsx
index 0801a34b48..2a9f8f88e2 100644
--- a/plugins/xcmetrics/src/components/StatusIcon/StatusIcon.test.tsx
+++ b/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();
   });
 });