FE: Add aria-hidden to decorative SVG icons (#1099)#1120
Conversation
Label icon-only close buttons in BoardSettingsModal, CardModalHeader, ColumnEditModal, LabelManagerModal, StarterPackCatalogModal that lacked an accessible name.
There was a problem hiding this comment.
Code Review
This pull request improves accessibility across several Vue components by adding aria-hidden="true" to decorative SVGs and introducing explicit aria-label attributes to close buttons. However, several buttons that contain these hidden SVGs rely solely on title attributes for screen readers, which is not a reliable method for providing accessible names. It is recommended to add explicit aria-label attributes to these buttons (such as the filter, keyboard shortcuts, board settings, edit column, and label action buttons) to ensure proper assistive technology support.
| title="Filter Cards (Press f)" | ||
| > | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true"> |
There was a problem hiding this comment.
Since the SVG is now marked as aria-hidden="true", this button will have no accessible text content for screen readers when filteredCardCount >= totalCardCount (since the filter count span is conditionally rendered). While the button has a title attribute, title is not a reliable accessible name across all assistive technologies and devices (especially touch screens). It is highly recommended to add an explicit aria-label="Filter Cards" to the parent <button> element, similar to how the close buttons were updated in this PR.
| title="Keyboard Shortcuts (Press ?)" | ||
| > | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true"> |
There was a problem hiding this comment.
Since the SVG is now marked as aria-hidden="true", this button has no accessible text content for screen readers. While the button has a title attribute, title is not a reliable accessible name across all assistive technologies and devices. It is highly recommended to add an explicit aria-label="Keyboard Shortcuts" to the parent <button> element.
| title="Board Settings" | ||
| > | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true"> |
There was a problem hiding this comment.
Since the SVG is now marked as aria-hidden="true", this button has no accessible text content for screen readers. While the button has a title attribute, title is not a reliable accessible name across all assistive technologies and devices. It is highly recommended to add an explicit aria-label="Board Settings" to the parent <button> element.
| title="Edit Column" | ||
| > | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true"> |
There was a problem hiding this comment.
Since the SVG is now marked as aria-hidden="true", this button has no accessible text content for screen readers. While the button has a title attribute, title is not a reliable accessible name across all assistive technologies and devices. It is highly recommended to add an explicit aria-label="Edit Column" to the parent <button> element.
| title="Edit" | ||
| > | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true"> |
There was a problem hiding this comment.
Since the SVG is now marked as aria-hidden="true", this button has no accessible text content for screen readers. While the button has a title attribute, title is not a reliable accessible name across all assistive technologies and devices. It is highly recommended to add an explicit aria-label="Edit label" to the parent <button> element.
| title="Delete" | ||
| > | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true"> |
There was a problem hiding this comment.
Since the SVG is now marked as aria-hidden="true", this button has no accessible text content for screen readers. While the button has a title attribute, title is not a reliable accessible name across all assistive technologies and devices. It is highly recommended to add an explicit aria-label="Delete label" to the parent <button> element.
Adversarial reviews (2 independent) + Fix Evidence — PR #1120Review 1 (self): confirmed the diff is accessibility-only; 16 decorative icons hidden, 5 close buttons given accurate Review 2 (independent): verified each hidden icon is decorative (blocked/due-date/drag/nav/empty-state icons all have sibling text or a parent label — correctly hidden) and the 5 close-button labels are accurate and needed. It found 5 real regressions: icon-only buttons whose only content was the now-hidden SVG and that had no
Fixes (commit
|
CI note — E2E Smoke flake (investigated, re-running)The E2E Smoke check failed on 1 of 152 tests:
Re-running the failed E2E job. No code change warranted. |
Resolves #1099.
Summary
Audited every inline
<svg>insrc/componentsandsrc/views, then addedaria-hidden="true"to decorative icons that lacked botharia-hiddenand arole. Applied per-SVG judgment (not a bulk edit). SVGs already handled (existingaria-hidden,role="img", dynamicv-binda11y, or anaria-hiddenwrapper parent) were left untouched.Decorative SVGs hidden (16 total)
board/BoardCanvas.vue— empty-state icon next to "No columns yet" textboard/BoardSettingsModal.vue— close icon (see labelled controls below)board/BoardToolbar.vue— 5 icons: back (button has aria-label), filter / keyboard-help / settings (buttons havetitle), labels / starter-packs (button hastitle+ visible text)board/CardItem.vue— 4 icons: drag handle + move-menu (buttons have aria-label), blocked badge (next to "Blocked" text), due-date (next to date text)board/CardModalHeader.vue— close icon (see labelled controls below)board/ColumnEditModal.vue— close icon (see labelled controls below)board/ColumnLane.vue— drag handle (aria-label) + edit (title) iconsboard/FilterPanel.vue— filter heading icon (next to "Filter Cards"), close icon (button has aria-label) — the original PR Add aria-labels to filter clear buttons in FilterPanel #1098 findingboard/LabelManagerModal.vue— close icon (see below), edit + delete icons (buttons havetitle), "Create New Label" icon (next to text)board/StarterPackCatalogModal.vue— close icon (see labelled controls below)KeyboardShortcutsHelp.vue— close icon (button has aria-label)common/SessionTimeoutWarning.vue— dismiss/close icon (button has aria-label)common/ToastContainer.vue— close icon (button has aria-label)views/BoardsListView.vue— empty-state icon next to "No boards" textIcon-only controls given a parent
aria-label(5)These close buttons had NO accessible name and the SVG was their sole content. Hiding the SVG alone would have left the control unlabelled, so I added an
aria-labelto the parent button andaria-hidden="true"to the icon:board/BoardSettingsModal.vue→aria-label="Close board settings"board/CardModalHeader.vue→aria-label="Close card editor"board/ColumnEditModal.vue→aria-label="Close column editor"board/LabelManagerModal.vue→aria-label="Close label manager"board/StarterPackCatalogModal.vue→aria-label="Close starter packs"Intentionally left untouched
common/SessionTimeoutWarning.vue:27andcommon/ToastContainer.vue:19/31/43/55— icons already inside anaria-hidden="true"wrapper<div>(parent handles it)ui/TdSelect.vue:46— icon inside anaria-hidden="true"wrapper<span>ui/TdSpinner.vue,paper/PaperConfidenceDial.vue,views/.../TodayCadence.vue— already havearia-hidden/role="img"+aria-label(multiline attrs)paper/PaperIcon.vue— dynamically bindsaria-hiddenorrole="img"+aria-labelviav-bindLoginView.vue,ProfileSettingsView.vue, inbox panels,TdTag/TdToast/TdInlineAlert— already carryaria-hiddenVerification
npm run typecheck— cleannpx eslint <14 changed files>— clean (vuejs-accessibility rules enforced)npx vitest --runfor all 14 related specs — 199 passed