Skip to content

fix(a11y): use aria-label instead of non-functional alt prop on filter icons#41742

Merged
eschutho merged 2 commits into
masterfrom
fix/a11y-filter-icon-alt-prop
Jul 3, 2026
Merged

fix(a11y): use aria-label instead of non-functional alt prop on filter icons#41742
eschutho merged 2 commits into
masterfrom
fix/a11y-filter-icon-alt-prop

Conversation

@eschutho

@eschutho eschutho commented Jul 3, 2026

Copy link
Copy Markdown
Member

WCAG rule

WCAG 2.1 SC 4.1.2 — Level A (Name, Role, Value).

What changed

Two icon-only controls in the native filters config modal (FilterTitleContainer.tsx's "remove filter" icon and ItemTitleContainer.tsx's "remove item" icon, both Icons.DeleteOutlined) were passing an alt prop to try to give them a contextual accessible name. alt isn't a recognized prop on these Icons.* components (built on the shared BaseIcon) — it's just spread onto the DOM as an inert attribute — so screen readers fell back to BaseIcon's generic auto-generated label ("delete") instead of the intended "Remove filter" / "Remove item" text. Swapped alt= for aria-label= at both call sites, which does take effect (confirmed via BaseIcon.tsx's prop-spread order for this icon type).

A third call site (DraggableFilter.tsx, Icons.Drag) had the same alt bug, but that icon renders through BaseIcon's customIcons branch, which has a different, pre-existing issue: caller props are spread onto the inner SVG rather than the outer role-bearing <span>, so an aria-label override there doesn't reach the accessible element either way. Left that one alone rather than papering over it — it needs a fix in BaseIcon.tsx itself, which affects many more call sites and is out of scope for this small fix.

Also updated FilterConfigPane.test.tsx's "remove filter" test, which queried the DOM by the old [alt=...] attribute selector, to query by [aria-label=...] instead.

Behavior

No visual or functional change — same click handlers, same icons, same test coverage. Only the accessible name computation changes.

Test plan

  • npx jest src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx — 5/5 passing
  • Manual: open a dashboard's native filters config modal, tab to a filter's "remove" (trash) icon with a screen reader running, verify it announces "Remove filter" (not "delete")

eschutho added 2 commits July 3, 2026 10:07
Self-review found: (1) Icons.Drag renders through BaseIcon's customIcons
branch, which spreads caller props onto the inner SVG, not the outer
span that carries role/aria-label — so overriding its aria-label has no
effect on the announced name. That's a separate, pre-existing BaseIcon.tsx
bug, out of scope here, so the DraggableFilter.tsx change is reverted.
(2) FilterConfigPane.test.tsx queried the DOM by the old `alt` attribute;
updated to query by `aria-label` to match the DeleteOutlined fixes, which
do work (antdEnhancedIcons branch spreads rest onto the labeled element).
@dosubot dosubot Bot added change:frontend Requires changing the frontend dashboard:native-filters Related to the native filters of the Dashboard design:accessibility Related to accessibility standards labels Jul 3, 2026
@bito-code-review

bito-code-review Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #1e1da8

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: e09cb74..f0a4466
    • superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx
    • superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx
    • superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@eschutho eschutho requested a review from richardfogaca July 3, 2026 10:23
@eschutho eschutho merged commit 0b14f1c into master Jul 3, 2026
67 of 68 checks passed
@eschutho eschutho deleted the fix/a11y-filter-icon-alt-prop branch July 3, 2026 17:39
rusackas pushed a commit that referenced this pull request Jul 3, 2026
The a11y rename of the filter delete icon updated
FilterConfigPane.test.tsx but not these two queries, which look the
icon up by its accessible name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rusackas pushed a commit to fnardin-maystreet/superset that referenced this pull request Jul 5, 2026
The a11y rename of the filter delete icon updated
FilterConfigPane.test.tsx but not these two queries, which look the
icon up by its accessible name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend dashboard:native-filters Related to the native filters of the Dashboard design:accessibility Related to accessibility standards size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants