fix: braille cursor issue and extra braille text at beginning in nvda eliminated

[soundaryakp1999]

Description

Moves role="application" from the <textarea> to the wrapper <div> in the Braille component to fix screen reader regressions.

Related Issues

Follow-up to #593

Changes Made

The role="application" attribute was previously placed directly on the braille <textarea>, which caused two regressions:

1. Screen reader announcement: NVDA announced "edit braille app" before the braille content instead of treating it as a plain edit field.
2. Cursor movement: Arrow keys (e.g. right arrow) did not move the cursor within the braille textarea, because role="application" on the textarea itself overrides its native form control behavior.

Moving role="application" to the parent <div> fixes both issues. The textarea retains its native editing behavior (cursor movement, standard announcements), while the parent's application mode still ensures keys like Escape are passed through to the web app rather than being intercepted by the screen reader.

Before: role="application" on <textarea> → "edit braille app" announced, cursor movement broken
After: role="application" on wrapper <div> → textarea announced normally, cursor works, Escape still passes through in one press

Checklist

• 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.

[claude]

Code Review

Thanks for this targeted fix! The motivation is clear and the ARIA reasoning is solid. I do have one significant finding to flag.

---

Bug: role="application" ends up on both elements

The PR description says "Moving role=\"application\" from the <textarea> to the wrapper <div>", but the diff only adds it to the <div> — it never removes it from the <textarea>.

After this PR lands, Braille.tsx will have:

// ✅ newly added
<div id={id} role="application">   
  <textarea
    ...
    role="application"   {/* ⚠️ still present, unchanged */}
  />
</div>

That means the two inline comments directly contradict each other:

• <div> comment: "Placed on the div (not the textarea)…"
• <textarea> comment: "Placed on the textarea (not the wrapper div)…"

Why this matters accessibility-wise:
role="application" on the <textarea> overrides the native textbox role, which is exactly what the PR says caused the regressions (NVDA announcing "edit braille app" and broken cursor movement). Keeping it there means both regressions survive the fix.

Suggested fix: Remove role="application" and its comment block from the <textarea>, keeping it only on the wrapper <div>. The parent's application landmark already ensures screen readers pass all keys (including Escape) through to the app.

---

Minor: Stale comment on <textarea>

If role="application" is removed from the textarea, its comment block ("role='application' ensures screen readers…") should be removed as well to avoid confusion.

---

Everything else looks good

• The accessibility reasoning in the <div> comment is accurate and well-explained.
• The change is minimal and focused.
• The PR title and description clearly explain the regression root cause.

Please remove role="application" from the <textarea> and I think this is ready to go.

[claude]

Code Review for PR 596 - Braille role=application placement fix

[claude]

What is Good

• The root-cause analysis is correct. Placing role=application directly on a native form control overrides its implicit ARIA role, causing NVDA to announce 'edit braille app' and breaking cursor movement with arrow keys. Moving it to the parent div restores native textarea semantics while the application landmark still intercepts Escape.
• The JSX comment clearly explains the why, which is what future maintainers need.
• The change is minimal: only the attribute placement and its associated comment change.

---

Concern 1 — Missing accessible name on the application landmark

Per WAI-ARIA 1.2 and the ARIA Authoring Practices Guide, role=application SHOULD have an accessible name so screen reader users navigating by landmarks can identify the region. Without one, NVDA/JAWS announces something generic like 'application' with no context.

Suggested fix: add aria-label='Braille display' (or aria-labelledby pointing to a heading) to the wrapper div.

Concern 2 — Rationale for the placement change is implicit

PR #593 placed role=application on the textarea to 'avoid suppressing browse-mode navigation for any future sibling content.' That concern is silently dropped here. A brief note in the comment — e.g. browse-mode for sibling content is not a current concern; the textarea is the sole interactive child — would help future reviewers understand this was a deliberate trade-off.

---

Minor / Questions

• Manual testing: The PR title says 'Escape key exits multipanel plot when braille mode is active.' Has this been verified with NVDA and JAWS in an actual multipanel scenario? Explicit confirmation is valuable given the back-and-forth between fix: escape key works in braille mode for subplot exit #593 and this PR.
• Test coverage: No tests cover the rendered attribute placement. A simple @testing-library/react assertion — expect(wrapper).toHaveAttribute('role', 'application') on the div, and a negative assertion on the textarea — would document the intended structure and guard against future regressions. Not a blocker, but worth a follow-up issue.

---

Summary

The fix is directionally correct and addresses real screen reader regressions from #593. The main actionable item before merging is adding an accessible name (aria-label or aria-labelledby) to the div with role=application to satisfy ARIA naming requirements. The rest are suggestions for maintainability and test coverage.

[claude]

Code Review

Note: A prior automated comment on this PR incorrectly claimed `role="application"` would remain on the `<textarea>` after this change — that is not true. The diff clearly removes it (and its comment block) from the textarea. The fix is complete.

---

What's correct

• Moving `role="application"` from the `<textarea>` to the wrapper `
  ` is the right call. The native `textbox` role is restored on the textarea, which fixes both regressions: the "edit braille app" announcement and broken arrow-key cursor movement.
• Adding `aria-label="Braille display"` to the application landmark satisfies the WAI-ARIA naming requirement for `role="application"` regions.
• The updated inline comment is accurate, explains the rationale, and explicitly acknowledges the browse-mode trade-off from fix: escape key works in braille mode for subplot exit #593.
• Change is minimal and focused — exactly what a regression fix should be.

---

Remaining concern: `<textarea>` has no accessible name

The `<textarea>` has no `aria-label`, `aria-labelledby`, or associated ``. Screen readers will announce it as "edit multiline" or similar with no context. This is a pre-existing issue not introduced by this PR, but it's worth a follow-up since we're already touching accessibility here.

Suggested follow-up (separate PR or issue):

<textarea
  aria-label="Braille output"
  ...
/>

---

Minor: no regression test for attribute placement

Given the back-and-forth between #593 and this PR, a simple `@testing-library/react` assertion would guard against this regressing again:

expect(screen.getByRole('application')).toBeInTheDocument(); // on div
expect(screen.getByRole('textbox')).not.toHaveAttribute('role', 'application'); // textarea clean

Not a blocker, but worth a follow-up issue.

---

Overall: The fix is correct, the ARIA reasoning is sound, and the previous feedback has been addressed. Good to merge once reviewers sign off.

[soundaryakp1999]

All major suggestions by claudebot have been implemented. I have tested the PR locally. @nk1408 Could you please test if visually braille mode is as before? @jooyoungseo @nk1408 I believe its ready to merge.

[nk1408]

yes @soundaryakp1999 , looks good to me visually. Merging

[xabilitylab]

🎉 This PR is included in version 3.62.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀