From c11105998c7795b95bac3f20e0db651ac36ee5fe Mon Sep 17 00:00:00 2001
From: Sean McPherson <git@seanmcp.com>
Date: Wed, 13 Nov 2024 10:09:33 -0500
Subject: [PATCH] PhosphorIcon: default role to img when aria-label is provided
 (#2358)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary:
When an `aria-label` is provided, there should be a `role` applied to the element to give it meaning to browsers and screenreaders. This PR defaults the `role` to `"img"` in `PhosphorIcon` when an `aria-label` is provided.

I'm not sure what our definitions are for "major", "minor", and "patch" releases. _I_ would consider this a `minor` release since we are changing the behavior (even if it is slight), but it is backwards compatible so `patch` might be fine.

Issue: WB-1795

## Test plan:
- Tests pass
- Axe is happy

Author: SeanMcP

Reviewers: jeresig

Required Reviewers:

Approved By: jeresig

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⏭️  dependabot

Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2358
---
 .changeset/dirty-dragons-fix.md                        |  5 +++++
 .../src/components/__tests__/phosphor-icon.test.tsx    | 10 ++++++++++
 .../src/components/phosphor-icon.tsx                   |  5 ++++-
 3 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/dirty-dragons-fix.md

diff --git a/.changeset/dirty-dragons-fix.md b/.changeset/dirty-dragons-fix.md
new file mode 100644
index 000000000..2e9b50f62
--- /dev/null
+++ b/.changeset/dirty-dragons-fix.md
@@ -0,0 +1,5 @@
+---
+"@khanacademy/wonder-blocks-icon": minor
+---
+
+defaults the role of PhosphorIcon to "img" when an aria-label is provided
diff --git a/packages/wonder-blocks-icon/src/components/__tests__/phosphor-icon.test.tsx b/packages/wonder-blocks-icon/src/components/__tests__/phosphor-icon.test.tsx
index 4b7294f42..8248d4af6 100644
--- a/packages/wonder-blocks-icon/src/components/__tests__/phosphor-icon.test.tsx
+++ b/packages/wonder-blocks-icon/src/components/__tests__/phosphor-icon.test.tsx
@@ -48,6 +48,16 @@ describe("PhosphorIcon", () => {
         );
     });
 
+    it("applies role=img when aria-label is provided", async () => {
+        // Arrange
+
+        // Act
+        render(<PhosphorIcon icon={Plus} aria-label="something" />);
+
+        // Assert
+        expect(screen.getByRole("img")).toBeInTheDocument();
+    });
+
     it("calls viewportPixelsForSize with size from props", async () => {
         // Arrange
         const viewPortPixelsForSizeSpy = jest.spyOn(
diff --git a/packages/wonder-blocks-icon/src/components/phosphor-icon.tsx b/packages/wonder-blocks-icon/src/components/phosphor-icon.tsx
index 890a2de51..d2f23d8cc 100644
--- a/packages/wonder-blocks-icon/src/components/phosphor-icon.tsx
+++ b/packages/wonder-blocks-icon/src/components/phosphor-icon.tsx
@@ -26,7 +26,8 @@ type Props = Pick<AriaProps, "aria-hidden" | "aria-label" | "role"> & {
     className?: string;
 
     /**
-     * The role of the icon.
+     * The role of the icon. Will default to `img` if an `aria-label` is
+     * provided.
      * @see https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA24
      */
     role?: "img";
@@ -89,6 +90,7 @@ export const PhosphorIcon = React.forwardRef(function PhosphorIcon(
         style,
         testId,
         className,
+        role,
         ...sharedProps
     } = props;
 
@@ -113,6 +115,7 @@ export const PhosphorIcon = React.forwardRef(function PhosphorIcon(
             ]}
             data-testid={testId}
             ref={ref}
+            role={role ?? sharedProps["aria-label"] ? "img" : undefined}
         />
     );
 });