A11Y: SVG icons should be hidden unless a label is provided #33059
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements accessibility improvements for SVG icons by ensuring they are hidden by default unless an accessible label is provided. It updates unit tests to match the new markup and modifies the icon rendering logic in the library to reflect the new aria attribute behavior.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| plugins/discourse-local-dates/test/javascripts/unit/local-date-builder-test.js | Updated expected HTML output to include aria-hidden attribute |
| app/assets/javascripts/discourse/tests/integration/helpers/d-icon-test.gjs | Updated expected HTML output for SVG icons with aria-hidden attribute |
| app/assets/javascripts/discourse/app/lib/icon-library.js | Adjusted SVG rendering logic to conditionally set aria attributes based on provided parameters |
| @@ -156,12 +156,10 @@ registerIconRenderer({ | |||
| const id = escape(handleIconId(icon)); | |||
| let html = `<svg class='${escape(iconClasses(icon, params))} svg-string'`; | |||
|
|
|||
There was a problem hiding this comment.
The change prioritizes 'aria-label' over 'label' for setting aria attributes. Consider adding a comment to clarify that this behavior aligns with the intended A11Y guidelines where an explicit 'aria-label' takes precedence, while a 'label' is handled separately for providing visible descriptions.
| // Prioritize 'aria-label' over 'label' for setting ARIA attributes. | |
| // This aligns with A11Y guidelines where 'aria-label' provides an explicit | |
| // programmatically accessible name, while 'label' is used for visible descriptions. |
Copilot uses AI. Check for mistakes.
This is fairly minor because usually SVGs aren't focusable, but this adds `aria-hidden='true'` to all SVGs by default unless either an `aria-label` or `label` is provided. This gets reported to us in automated tests from time to time. So now: 1. SVG: `aria-hidden="true"` (new behavior, safe default when no label is provided) 2. SVG with `label` param: `aria-hidden="true"` and `sr-only` label is provided 3. SVG with `aria-label` param: `aria-hidden="false"` and `aria-label` is provided
This is fairly minor because usually SVGs aren't focusable, but this adds
aria-hidden='true'to all SVGs by default unless either anaria-labelorlabelis provided. This gets reported to us in automated tests from time to time.So now:
aria-hidden="true"(new behavior, safe default when no label is provided)labelparam:aria-hidden="true"andsr-onlylabel is providedaria-labelparam:aria-hidden="false"andaria-labelis provided