StateLabel: Add labels to icon (#4764)

* fix accessibility issue first * stronger types * add tests, update snapshots * remove unused icon * remove unrelated changes * remove unrelated changes * run lint locally (sigh) * Create wild-actors-jam.md * bring back eslint-disable-line * add not planned for issueClosedNotPlanned * have to add an awkward member to the types * update snapshots
primer · Jul 24, 2024 · 533f997 · 533f997
1 parent bd362cb
commit 533f997
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 9 deletions.
diff --git a/.changeset/wild-actors-jam.md b/.changeset/wild-actors-jam.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+StateLabel: Differentiate issue and pull request labels for screen readers
diff --git a/packages/react/src/StateLabel/StateLabel.tsx b/packages/react/src/StateLabel/StateLabel.tsx
@@ -33,6 +33,19 @@ const octiconMap = {
   unavailable: AlertIcon,
 }
 
+const labelMap: Record<keyof typeof octiconMap, 'Issue' | 'Issue, not planned' | 'Pull request' | ''> = {
+  issueOpened: 'Issue',
+  pullOpened: 'Pull request',
+  issueClosed: 'Issue',
+  issueClosedNotPlanned: 'Issue, not planned',
+  pullClosed: 'Pull request',
+  pullMerged: 'Pull request',
+  draft: 'Pull request',
+  issueDraft: 'Issue',
+  pullQueued: 'Pull request',
+  unavailable: '',
+}
+
 const colorVariants = variant({
   prop: 'status',
   variants: {
@@ -120,7 +133,15 @@ function StateLabel({children, status, variant: variantProp = 'normal', ...rest}
   return (
     <StateLabelBase {...rest} variant={variantProp} status={status}>
       {/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */}
-      {status && <Octicon {...octiconProps} icon={octiconMap[status] || QuestionIcon} sx={{mr: 1}} />}
+      {status && (
+        <Octicon
+          {...octiconProps}
+          // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+          icon={octiconMap[status] || QuestionIcon}
+          aria-label={labelMap[status]}
+          sx={{mr: 1}}
+        />
+      )}
       {children}
     </StateLabelBase>
   )

diff --git a/packages/react/src/StateLabel/__tests__/StateLabel.test.tsx b/packages/react/src/StateLabel/__tests__/StateLabel.test.tsx
@@ -43,4 +43,14 @@ describe('StateLabel', () => {
   it('renders children', () => {
     expect(render(<StateLabel status="issueOpened">hi</StateLabel>)).toMatchSnapshot()
   })
+
+  it('adds label to icon', () => {
+    const screen1 = HTMLRender(<StateLabel status="issueOpened">Open</StateLabel>)
+    expect(screen1.getByLabelText('Issue')).toBeInTheDocument() // svg
+    expect(screen1.getByText('Open')).toBeInTheDocument() // text
+
+    const screen2 = HTMLRender(<StateLabel status="pullMerged">Merged</StateLabel>)
+    expect(screen2.getByLabelText('Pull request')).toBeInTheDocument() // svg
+    expect(screen2.getByText('Merged')).toBeInTheDocument() // text
+  })
 })
diff --git a/packages/react/src/StateLabel/__tests__/__snapshots__/StateLabel.test.tsx.snap b/packages/react/src/StateLabel/__tests__/__snapshots__/StateLabel.test.tsx.snap
@@ -32,11 +32,12 @@ exports[`StateLabel renders children 1`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Issue"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -91,11 +92,12 @@ exports[`StateLabel respects the status prop 1`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Issue"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -149,11 +151,12 @@ exports[`StateLabel respects the status prop 2`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Issue"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -207,11 +210,12 @@ exports[`StateLabel respects the status prop 3`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Issue, not planned"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -262,11 +266,12 @@ exports[`StateLabel respects the status prop 4`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Pull request"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -317,11 +322,12 @@ exports[`StateLabel respects the status prop 5`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Pull request"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -372,11 +378,12 @@ exports[`StateLabel respects the variant prop 1`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Issue"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",
@@ -430,11 +437,12 @@ exports[`StateLabel respects the variant prop 2`] = `
   className="c0"
 >
   <svg
-    aria-hidden="true"
+    aria-label="Issue"
     className="c1"
     fill="currentColor"
     focusable="false"
     height={16}
+    role="img"
     style={
       {
         "display": "inline-block",