Skip to content

Conversation

@ychhabra-eightfold
Copy link
Collaborator

@ychhabra-eightfold ychhabra-eightfold commented Sep 17, 2025

SUMMARY:

This PR implements two key accessibility improvements for the DateTimePicker Range Picker component:

1. Focus Trap Support

  • Added trapFocus prop support to OcRangePicker (defaults to false to prevent regressions)
  • Implemented FocusTrap component wrapper when enabled
  • Added proper keyboard handling (Escape key) and mouse event prevention
  • Updated range picker generator to pass through trapFocus prop

2. Fixed Arrow Key Focus Behavior

  • Root Issue: Range picker focus remained stuck on header elements when navigating with arrow keys, preventing proper date announcements for screen readers
  • Root Cause: useCellProps hook returned undefined for range pickers, disabling focus management on date cells
  • Solution: Modified useCellProps to always return cell props, enabling proper focus shifts to date cells
  • Added missing partialRef to range picker's PartialContext for complete focus management

3. Announcement Support

  • Added announceArrowKeyNavigation prop support for range picker when dropdown opens
  • Screen reader now properly announces date changes during arrow key navigation

Result: Range picker now has the same accessibility experience as single picker - arrow key navigation properly shifts focus to date cells and announces changes.

GITHUB ISSUE (Open Source Contributors)

N/A - Internal accessibility improvement

JIRA TASK (Eightfold Employees Only):

[Please fill in JIRA task number]

CHANGE TYPE:

  • Feature Pull Request
  • Bugfix Pull Request

TEST COVERAGE:

  • Tests for this change already exist
  • I have added unittests for this change

TEST PLAN:

Manual Testing

  1. Focus Trap (when enabled):

    • Open range picker with trapFocus={true}
    • Press Tab - focus should be trapped within the picker
    • Press Escape - picker should close and trap should disable
  2. Arrow Key Focus Behavior:

    • Open range picker
    • Press Tab to focus header elements
    • Press arrow keys - focus should shift to date cells (previously stayed on header)
    • Screen reader should announce date changes during navigation
  3. Announcement Support:

    • Enable screen reader
    • Open range picker with announceArrowKeyNavigation={true}
    • Should hear announcement about arrow key navigation availability

Automated Testing

  • All existing tests pass
  • No regressions introduced
  • Focus behavior properly maintained
  • Default trapFocus={false} prevents any breaking changes

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 17, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@tejaswini-s-c-eightfold

I have verified the announcement with NVDA and without NVDA

  1. With NVDA Screen reader the arrow navigation is not working
  2. Without NVDA Screen reader it is working as I am able to navigate using the arrow keys

@ychhabra-eightfold @pmahajan-eightfold

Arrow.Navigation.for.Date.picker.mp4

Copy link
Contributor

@mfazil-eightfold mfazil-eightfold left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Sep 19, 2025

Codecov Report

❌ Patch coverage is 59.25926% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.45%. Comparing base (cc4688f) to head (15d096b).

Files with missing lines Patch % Lines
...mponents/DateTimePicker/Internal/OcRangePicker.tsx 47.61% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
- Coverage   84.48%   84.45%   -0.04%     
==========================================
  Files        1104     1104              
  Lines       20801    20824      +23     
  Branches     7877     7893      +16     
==========================================
+ Hits        17573    17586      +13     
- Misses       3140     3150      +10     
  Partials       88       88              

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

@ychhabra-eightfold ychhabra-eightfold merged commit 4b42384 into EightfoldAI:main Sep 19, 2025
4 of 5 checks passed
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