CODAP-575: add aria-label to tool shelf and inspector panel buttons

[kswenson]

Summary

• Add aria-label to all toolbar buttons (Tables, Graph, Map, Slider, Calc, Text, Plugins, Undo, Redo, Tiles, Guide)
• Add aria-label to InspectorButton and InspectorMenu components, covering all tile inspector panels
• For icon-only buttons (e.g., text inspector), fall back to tooltip text with parenthetical keyboard shortcuts stripped for clean screen reader announcements

Fixes CODAP-575

Test plan

• Verify toolbar buttons have correct aria-label in browser dev tools
• Verify inspector panel buttons (graph, table, card, map, text, slider, web view) have correct aria-label
• Verify text inspector icon-only buttons get clean labels without keyboard shortcut syntax
• Test with a screen reader (VoiceOver) to confirm buttons are announced correctly

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.27%. Comparing base (436e61b) to head (4f9786d).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
v3/src/components/inspector-panel.tsx	66.66%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (436e61b) and HEAD (4f9786d). Click for more details.

HEAD has 15 uploads less than BASE

Flag	BASE (436e61b)	HEAD (4f9786d)
cypress	16	1

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2433       +/-   ##
===========================================
- Coverage   85.51%   69.27%   -16.25%     
===========================================
  Files         758      758               
  Lines       42137    42140        +3     
  Branches    10428    10342       -86     
===========================================
- Hits        36034    29193     -6841     
- Misses       6088    12922     +6834     
- Partials       15       25       +10     

Flag	Coverage Δ
cypress	39.26% <66.66%> (-29.92%)	⬇️
jest	57.04% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR improves accessibility by adding explicit aria-label attributes to tool shelf and inspector panel buttons so screen readers can announce meaningful names (especially for icon-only controls).

Changes:

• Add aria-label to the generic ToolShelfButton and to several menu-style tool shelf buttons (tiles list, plugins, guide, case table).
• Add aria-label support to InspectorButton and InspectorMenu, including cleanup of tooltip-derived labels by stripping trailing parenthetical shortcuts.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
v3/src/components/tool-shelf/tool-shelf-button.tsx	Adds aria-label to the base tool shelf button component.
v3/src/components/tool-shelf/tiles-list-button.tsx	Adds aria-label to the tiles list menu button.
v3/src/components/tool-shelf/plugins-button.tsx	Adds aria-label to the plugins menu button.
v3/src/components/tool-shelf/guide-button.tsx	Adds aria-label to the guide menu button.
v3/src/components/inspector-panel.tsx	Adds aria-label to inspector buttons/menus; strips trailing shortcut text from tooltip-based labels.
v3/src/components/case-table/case-table-tool-shelf-button.tsx	Adds aria-label to the case table menu button.

Comments suppressed due to low confidence (1)

v3/src/components/tool-shelf/tool-shelf-button.tsx:33

• ToolShelfButton receives label/hint as already-translated UI strings (e.g. label={t(labelKey)} in tool-shelf.tsx). Calling t(label)/t(hint) again for aria-label/title is redundant and can produce incorrect output if the translated string happens to match another translation key. Use the label/hint props directly for aria-label and title (and consider doing the same for ToolShelfButtonTag).

      as='button'
      aria-label={t(label)}
      title={t(hint)}
      disabled={disabled}
      onClick={onClick}
      data-testid={`tool-shelf-button-${label.toLowerCase()}`}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cypress]

codap-v3    Run #10456

Run Properties:   Passed #10456  •  56330066d1: Increment the build number

Project	codap-v3
Branch Review	main
Run status	Passed #10456
Run duration	02m 19s
Commit	56330066d1: Increment the build number
Committer	github-actions[bot]
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	4
View all changes introduced in this branch ↗︎

[emcelroy]

While these changes generally look OK and should technically work OK, I don't think adding aria-label for buttons that contain text, and setting that aria-label value to the same value is the right thing to do. My understanding is that unless a button contains no text or there's a need to provide a label that differs from the text it contains, there's no reason for adding aria-label. And generally, unnecessary use of ARIA is discouraged: The First Rule of ARIA Use

It does make sense to add aria-label for buttons that don't have text (like those that don't have a defined label value in inspector-panel.tsx) or have text that we want to adjust for screen readers (like where you're stripping the keyboard shortcuts).

From developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-label

"By default, a button's accessible name is the content between the opening and closing tags."

These buttons do contain other elements besides the text, but they're (at least currently) "invisible" to screen readers because they don't have alt attributes. So just the button text will be used as the label. One way to check that is to inspect the button in Chrome's Dev Tools and view the Accessibility tab. The "Name" value is the label. If you do that on the main branch you'll see "Table" for the Table button, for example, even though there's no aria-label used there.

So, I would suggest removing aria-label wherever it's clearly redundant. Again, it's probably OK as-is, but I still don't think it's the right thing to do. If you have a counter argument, I'm open to hearing it, though.

[kswenson]

So, I would suggest removing aria-label wherever it's clearly redundant. Again, it's probably OK as-is, but I still don't think it's the right thing to do. If you have a counter argument, I'm open to hearing it, though.

I asked Claude about this. The problem is how one interprets "clearly redundant". None of the usages here are <button>Label</button>. Sometimes the user-visible label is several layers deep in a Chakra <MenuButton>. When I asked Claude whether any of these fell into the "clearly redundant" camp, it's answer was something to the effect of "it depends on the screen reader". So that was why I went with assigning an aria-label in these cases. That said, I have since downloaded a different plugin. I can see if it has anything different to say.

[kswenson]

Accessibility Audit: aria-label Redundancy Analysis

I ran a thorough accessibility audit on these changes, tracing through Chakra UI's internal rendering to understand the actual DOM structure and accessible name computation. Here's the summary:

Necessary — must keep

InspectorButton (icon-only) and InspectorMenu (icon-only) — e.g. the text formatting buttons (bold, italic, underline, etc.). These have no visible text at all. Without aria-label, they would have no accessible name, which is a WCAG 4.1.2 violation. The regex stripping keyboard shortcuts from tooltips ("bold (⌘-b)" → "bold") is a nice touch.

Redundant but harmless

All tool shelf buttons (ToolShelfButton, CaseTableToolShelfButton, GuideButton, PluginsButton, TilesListShelfButton) and InspectorButton/InspectorMenu when a label prop is provided.

These buttons all render visible text via <ToolShelfButtonTag> or <span className="inspector-button-label">. The accessible name computation algorithm (AccName spec, rule 2F) recursively walks the subtree of any <button> element. Chakra's internal wrapper <span> elements (in MenuButton and Tag) don't have aria-hidden or role="presentation", so the text content is reliably found by screen readers.

Since the aria-label and visible text are always derived from the same t(...) call or label variable, there's no WCAG 2.5.3 (Label in Name) risk and minimal maintenance divergence risk.

The reviewer's concern is technically correct — these aria-label attributes are redundant. However, keeping them is a valid "defensive accessibility" practice (explicit over implicit), and they cause no harm. It's a team style call, not a correctness issue.

Minor suggestion

InspectorMenu's tooltip fallback doesn't strip parenthetical keyboard shortcuts the way InspectorButton does. Not a problem today (no InspectorMenu tooltips contain shortcuts), but worth making consistent.

[kswenson]

So, I would suggest removing aria-label wherever it's clearly redundant. Again, it's probably OK as-is, but I still don't think it's the right thing to do. If you have a counter argument, I'm open to hearing it, though.

I asked Claude about this. The problem is how one interprets "clearly redundant". None of the usages here are <button>Label</button>. Sometimes the user-visible label is several layers deep in a Chakra <MenuButton>. When I asked Claude whether any of these fell into the "clearly redundant" camp, it's answer was something to the effect of "it depends on the screen reader". So that was why I went with assigning an aria-label in these cases. That said, I have since downloaded a different plugin. I can see if it has anything different to say.

Addressed in the latest commit.

[emcelroy]

So, I would suggest removing aria-label wherever it's clearly redundant. Again, it's probably OK as-is, but I still don't think it's the right thing to do. If you have a counter argument, I'm open to hearing it, though.

I asked Claude about this. The problem is how one interprets "clearly redundant". None of the usages here are <button>Label</button>. Sometimes the user-visible label is several layers deep in a Chakra <MenuButton>. When I asked Claude whether any of these fell into the "clearly redundant" camp, it's answer was something to the effect of "it depends on the screen reader". So that was why I went with assigning an aria-label in these cases. That said, I have since downloaded a different plugin. I can see if it has anything different to say.

Addressed in the latest commit.

Thanks 👍 I think that's better, though I do understand it's open to interpretation.

[emcelroy]

Looks good 👍

For the record, I wouldn't be totally opposed to using aria-label for all the buttons, and think it's reasonable to at least consider it. I always remember that First Rule of ARIA, though, and tend to think it's best not to use it unless it's necessary.