Skip to content

Conversation

Ovgodd
Copy link
Collaborator

@Ovgodd Ovgodd commented Sep 18, 2025

Purpose

Improve accessibility by hiding decorative icons from assistive technologies using aria-hidden. This prevents screen readers from announcing icons that are purely visual

This solves issue : 1390

Proposal

  • Add aria-hidden logic to based on presence of ARIA labels
  • Ensure decorative icons default to aria-hidden="true" when appropriate
  • Respect existing ARIA attributes when explicitly set by component props

@Ovgodd Ovgodd requested a review from AntoLC September 18, 2025 06:33
@Ovgodd Ovgodd self-assigned this Sep 18, 2025
@Ovgodd Ovgodd force-pushed the fix/1390-hide-decorative-icons branch from f61a9df to 5f8de1d Compare September 18, 2025 06:34
@Ovgodd Ovgodd marked this pull request as ready for review September 18, 2025 06:34
Copy link

github-actions bot commented Sep 18, 2025

Size Change: +63 B (0%)

Total Size: 3.65 MB

Filename Size Change
apps/impress/out/_next/static/c7f24d5e/_buildManifest.js 0 B -863 B (removed) 🏆
apps/impress/out/_next/static/d3c0647a/_buildManifest.js 863 B +863 B (new file) 🆕

compressed-size-action

Copy link
Collaborator

@AntoLC AntoLC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but I think you will have some tests e2e to fix, the icon will not be accessible in some places.

@Ovgodd
Copy link
Collaborator Author

Ovgodd commented Sep 18, 2025

Looks good, but I think you will have some tests e2e to fix, the icon will not be accessible in some places.

Yep indeed !

@Ovgodd Ovgodd force-pushed the fix/1390-hide-decorative-icons branch 3 times, most recently from 2047256 to 613b7e7 Compare September 18, 2025 13:19
@Ovgodd Ovgodd requested a review from AntoLC September 18, 2025 13:41
@Ovgodd
Copy link
Collaborator Author

Ovgodd commented Sep 18, 2025

Looks good, but I think you will have some tests e2e to fix, the icon will not be accessible in some places.

All good, all e2e tests pass.

improves accessibility by reducing screen reader noise from icon elements

Signed-off-by: Cyril <c.gromoff@gmail.com>
@Ovgodd Ovgodd force-pushed the fix/1390-hide-decorative-icons branch from 613b7e7 to e5e5fba Compare September 22, 2025 06:32
@Ovgodd Ovgodd merged commit e5e5fba into main Sep 22, 2025
21 of 22 checks passed
@Ovgodd Ovgodd deleted the fix/1390-hide-decorative-icons branch September 22, 2025 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants