Skip to content

feat(a11y): keyboard navigation and focus management - #1648

Open
AbhishekChorotiya wants to merge 2 commits into
feat/a11y-form-controlsfrom
feat/a11y-keyboard-focus
Open

feat(a11y): keyboard navigation and focus management#1648
AbhishekChorotiya wants to merge 2 commits into
feat/a11y-form-controlsfrom
feat/a11y-keyboard-focus

Conversation

@AbhishekChorotiya

@AbhishekChorotiya AbhishekChorotiya commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

This PR improves keyboard navigation and focus management for the payment experience. It adds shared keyboard activation and focus primitives, applies clearer expanded/dialog state where relevant, and keeps non-native interactive controls operable with keyboard expectations that match native controls.

The user impact is smoother keyboard traversal, clearer assistive-technology context for dialogs and expandable controls, and fewer focus traps or dead ends during payment selection.

This PR is stacked after the form-control accessibility PR because it reuses the shared accessibility helper module introduced there.

Closes #1647

How did you test it?

Validated as part of the completed accessibility stack. The checks cover the combined flow after all stacked PRs are applied, including keyboard and focus behavior exercised during the local accessibility smoke checks.

  • Ran npm run re:build on the completed accessibility stack.
  • Ran npm run test:hooks on the completed accessibility stack.
  • Ran npm run build on the completed accessibility stack.
  • Ran committed-range whitespace validation on the completed accessibility stack.

Checklist

  • I ran npm run re:build
  • I reviewed submitted code
  • I added unit tests for my changes where possible

@semanticdiff-com

Copy link
Copy Markdown

Review changes with  SemanticDiff

@AbhishekChorotiya
AbhishekChorotiya force-pushed the feat/a11y-keyboard-focus branch from 7a20533 to f13e8f7 Compare June 29, 2026 08:37
@AbhishekChorotiya
AbhishekChorotiya force-pushed the feat/a11y-form-controls branch from 0451104 to 2d38c44 Compare June 29, 2026 08:37
Add ARIA semantics across all text inputs and selects; required state wired from
fieldConfig.isRequired; ARIA on bypassing selects/inputs. Localization: accessibility
locale dictionary scaffold + the keys used here (cardNetworkLabel, morePaymentMethodsLabel,
yearLabel, monthLabel, optionTabLabel). Reuse: shared LiveError component for announced errors.
Focus hooks (useFocusTrap/useEscapeKey/useReturnFocus); Modal dialog + focus trap +
Escape + accessible close; accordion roving tabindex + arrow-key radiogroup; Checkbox
Space. ARIA labels localized (closeLabel, dialogLabel, paymentMethodsGroupLabel).
Reuse: shared AccessibilityUtils (focus/querySelectorAllWithin/activeElement +
focusableSelector) consumed by the focus hooks and accordion, removing duplicated externals.
@XyneSpaces

Copy link
Copy Markdown

Review Summary

PR: #1648 - feat(a11y): keyboard navigation and focus management

Findings

  1. Good: New FocusHooks.res provides reusable focus management primitives (useFocusTrap, useEscapeKey, useReturnFocus).

  2. Good: Accordion component implements roving tabindex pattern with proper ArrowUp/ArrowDown keyboard navigation.

  3. Good: Modal component includes role="dialog", ariaModal={true}, and ariaLabel for proper screen reader announcement.

  4. Issue: The focusableSelector in AccessibilityUtils.res uses a hardcoded CSS selector string:

    let focusableSelector = "button, [href], input, select, textarea, [tabindex]:not([tabindex='-1'])"

    Suggestion: Consider also including audio[controls], video[controls], and [contenteditable] elements for completeness per WCAG guidelines.

  5. Good: Locale strings for closeLabel, dialogLabel, and paymentMethodsGroupLabel are properly added across locale files.

No blocking issues. The implementation follows ARIA authoring practices for keyboard navigation.

@XyneSpaces

Copy link
Copy Markdown

⚠️ Verify postMessage origin validation in focus delegation

The keyboard navigation and focus management PR introduces cross-iframe communication. Ensure any postMessage calls for focus delegation validate the target origin — do not use "*" as targetOrigin.

Flag any instances of:

Window.postMessage(message, "*")

Use GlobalVars.hyperSwitchSdkUrl or a whitelist of allowed origins instead.

@XyneSpaces

Copy link
Copy Markdown

⚠️ New focus management hook missing error boundaries

The new UseFocusManager.res hook manages DOM focus and keyboard navigation. If any of the ref callbacks or focus calls fail (e.g., element removed from DOM between render and effect), the error is unhandled. Consider wrapping focus attempts in try/catch and logging failures, or document the invariant that refs are always valid when focus is called.

Also verify this hook is compatible with React 18's concurrent features — synchronous focus management during render can cause issues with time-slicing.

} else {
None
}
}, [enabled])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}, [enabled])
}, (enabled, onEscape))

Comment thread src/Components/Modal.res
className="p-4 flex justify-end self-end mb-4 cursor-pointer"
onClick={_ => closeModal()}>
onClick={_ => closeModal()}
ariaLabel={localeString.closeLabel}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if locale support is working in modal flows or not.

#"false"
}

@send external focus: Dom.element => unit = "focus"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image we have same in cardUtils.res

@AbhishekChorotiya
AbhishekChorotiya force-pushed the feat/a11y-form-controls branch 3 times, most recently from ab7307b to cc05cab Compare July 21, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants