Skip to content

feat: Add ariaLabel property to icon component#3244

Merged
avinashbot merged 2 commits intomainfrom
feat-icon-aria-label
Feb 5, 2025
Merged

feat: Add ariaLabel property to icon component#3244
avinashbot merged 2 commits intomainfrom
feat-icon-aria-label

Conversation

@avinashbot
Copy link
Member

@avinashbot avinashbot commented Feb 3, 2025

Description

+   /**
+    * Specifies alternate text for the icon. We recommend that you provide this for accessibility.
+    */
+   ariaLabel?: string;

    /**
-    * Specifies alternate text for a custom icon (using the `url` attribute). We recommend that you provide this for accessibility.
+    * Specifies alternate text for a custom icon (using the `url` attribute).
     * This property is ignored if you use a predefined icon or if you set your custom icon using the `svg` slot.
     *
+    * @deprecated Use `ariaLabel` instead.
     */
    alt?: string;

Motivation

If you want to have an accessible label for your icon (assuming you don't have visible text next to it), you currently have to do this:

<span role="img" aria-label="Success">
  <Icon name="status-positive" />
</span>

Boo! Annoying! The current approach feels manual and seriously lacks discoverability - people can't find an ariaLabel property, and sometimes end up (understandably) misusing the alt property. We could really just let people do this instead:

<Icon name="status-positive" ariaLabel="Success" />

This alone doesn't fix the fact that alt is still confusing, so I also deprecated alt (ariaLabel will be used if provided, but we'll still fall back to "alt" for <img> only).

Related links, issue #, if available: n/a

How has this been tested?

Unit tests. Ignore all the force pushes, I was too lazy to run unit tests on my laptop.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.44%. Comparing base (3cdecff) to head (33c1fcf).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3244      +/-   ##
==========================================
+ Coverage   96.42%   96.44%   +0.01%     
==========================================
  Files         790      791       +1     
  Lines       22322    22568     +246     
  Branches     7255     7792     +537     
==========================================
+ Hits        21524    21765     +241     
- Misses        791      796       +5     
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avinashbot avinashbot force-pushed the feat-icon-aria-label branch from 8eaf2d3 to 4a66db6 Compare February 4, 2025 15:06
@avinashbot avinashbot changed the title feat: Add ariaLabel property to icon feat: Add ariaLabel property to icon component Feb 4, 2025
@avinashbot avinashbot force-pushed the feat-icon-aria-label branch 2 times, most recently from 8abf280 to 7103c0b Compare February 4, 2025 16:05
@avinashbot avinashbot force-pushed the feat-icon-aria-label branch from 7103c0b to 33c1fcf Compare February 4, 2025 16:15
@avinashbot avinashbot marked this pull request as ready for review February 4, 2025 16:34
@avinashbot avinashbot requested a review from a team as a code owner February 4, 2025 16:34
@avinashbot avinashbot requested review from georgylobko and removed request for a team February 4, 2025 16:34
@avinashbot
Copy link
Member Author

Just wanted to be sure before merging, dry-run build (7222839735) is successful.

@avinashbot avinashbot added this pull request to the merge queue Feb 5, 2025
Merged via the queue into main with commit f8a00dd Feb 5, 2025
41 checks passed
@avinashbot avinashbot deleted the feat-icon-aria-label branch February 5, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants