Skip to content

fix: address JAWS/NVDA not recognizing image assets#565

Merged
nk1408 merged 1 commit intomainfrom
fix/img-tag
Feb 26, 2026
Merged

fix: address JAWS/NVDA not recognizing image assets#565
nk1408 merged 1 commit intomainfrom
fix/img-tag

Conversation

@nk1408
Copy link
Collaborator

@nk1408 nk1408 commented Feb 26, 2026

Pull Request

Description

Summary

Screen readers like NVDA and JAWS could not discover MAIDR charts using quick-navigation keys (e.g. pressing "g" to jump to the next graphic), and single-letter hotkeys (b, t, s, r, g, etc.) were intercepted by the screen reader instead of reaching MAIDR's keybinding system.

Root causes:

  1. Missing role="img" at page load — The React refactor removed the old "throwaway Controller" pattern that set role="img" on mount. The plot
    rendered bare with no ARIA role, making it invisible to screen reader graphics navigation.
  2. role="graphics-document" on focus-in — This role keeps NVDA/JAWS in browse mode, where single-letter keys are consumed by the screen reader for quick-navigation before they reach JavaScript.

Fix:

  • Set role="img" + aria-label + title at React render time so charts are discoverable by screen readers immediately on page load
  • Switch to role="application" on focus-in, which forces NVDA/JAWS into forms/focus mode so all keys pass through to hotkeys-js
  • Revert to role="img" on focus-out so graphics navigation continues to work

This restores the same role-switching behavior (img → application → img) that existed before the React migration.

Checklist

  • I have read the Contributor Guidelines.
  • I have performed a self-review of my own code and ensured it follows the project's coding standards.
  • I have tested the changes locally following ManualTestingProcess.md, and all tests related to this pull request pass.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation, if applicable.
  • I have added appropriate unit tests, if applicable.

Additional Notes

@claude
Copy link

claude bot commented Feb 26, 2026

PR Review: feat: address JAWS/NVDA not recognizing image assets

Great fix for a real accessibility regression. The root cause analysis is accurate and the solution approach (role="img" at render → role="application" on focus → role="img" on blur) is the right pattern for JAWS/NVDA forms-mode compatibility. A few issues to address:


Bug: getInitialInstruction diverges from Context.getInstruction for multiline plots

The new static function in maidr-component.tsx uses the raw layer type string, but Context.getInstruction computes a richer plotType (e.g., "multiline" with a group count). For a multi-line chart:

  • Initial aria-label (static function): "This is a maidr plot of type: line. Click to activate..."
  • Post-focus aria-label (Controller): "This is a maidr plot of type: multiline with 3 groups. Click to activate..."

A screen reader user navigating with g will hear "line", then on focus the Controller overwrites it with "multiline with 3 groups" — a confusing inconsistency. The function should either accept these limitations (and document them), or derive the type more accurately.

This is also verifiable against the existing constants: TestConstants.MULTI_LINEPLOT_INSTRUCTION_TEXT expects "multiline with 3 groups" which getInitialInstruction cannot produce from raw layer data.


Formatting inconsistency between getInitialInstruction and Context.getInstruction

Context.getInstruction for the 'figure' case includes embedded newlines and indentation whitespace (template literal formatting), while getInitialInstruction produces a flat single-line string. These produce different announcements for multi-subplot figures.


Code duplication in removeInstruction

The last three lines of both branches of the if/else are identical:

this.plot.removeAttribute(Constant.TITLE);
this.plot.setAttribute(Constant.ROLE, Constant.APPLICATION);
this.plot.tabIndex = 0;

These can be hoisted outside the conditional, making future changes safer and the code easier to read.


addInstruction calls getInstruction() twice

private addInstruction(): void {
  this.plot.setAttribute(Constant.ARIA_LABEL, this.getInstruction()); // call 1
  this.plot.setAttribute(Constant.TITLE, this.getInstruction());      // call 2

Store the result in a local variable to avoid the redundant call.


Constant.IMAGE reused as an ARIA role value

Constant.IMAGE = 'img' was originally defined as an HTML tag name constant. Reusing it as an ARIA role value (role="img") works because the string happens to match, but it is semantically misleading — compare to the newly added Constant.APPLICATION which was purpose-defined for its role use. Consider adding Constant.ROLE_IMG = 'img' or a clarifying comment.


VoiceOver regression risk with role="application"

The removed role="graphics-document" was part of the WAI-ARIA Graphics Module and had good VoiceOver (macOS/iOS) support. role="application" solves JAWS/NVDA forms mode entry, but VoiceOver does not use a "forms mode" equivalent — applying role="application" disables VoiceOver's rotor and standard virtual cursor navigation within the element. The PR description focuses on JAWS/NVDA testing. Please explicitly confirm VoiceOver behavior is acceptable before merge, since this could be a significant regression for macOS/iOS users.


Missing E2E tests for role transitions

No tests assert the role lifecycle:

  • role="img" present on the plot div before focus
  • role="application" present during focus
  • role="img" restored after blur

Playwright's getAttribute('role') makes these straightforward to add and would protect against future regressions of this fix.


examples/test-role-switch.html references ../dist/ build artifacts

The file references ../dist/maidr.css and ../dist/maidr.js which only exist after npm run build. A comment at the top noting this prerequisite would help contributors using it for manual testing.


Summary

The core accessibility fix is correct and well-motivated. The primary concern is getInitialInstruction producing different text than Context.getInstruction for multi-line and multi-subplot charts, which creates an inconsistent screen reader experience for users navigating by graphics quick-key. VoiceOver impact should be explicitly confirmed. The code duplication and double-call in display.ts are minor cleanup items.

@nk1408 nk1408 changed the title feat: address JAWS/NVDA not recognizing image assets fix: address JAWS/NVDA not recognizing image assets Feb 26, 2026
@nk1408 nk1408 merged commit c41495d into main Feb 26, 2026
7 checks passed
@nk1408 nk1408 deleted the fix/img-tag branch February 26, 2026 22:38
@xabilitylab
Copy link
Collaborator

🎉 This PR is included in version 3.53.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@xabilitylab xabilitylab added the released For issues/features released label Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released For issues/features released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants