priceline / design-system

Priceline.com Design System
https://priceline.github.io/design-system/
MIT License
722 stars 119 forks source link

feat(UXPT-6345): add a new parameter 'shouldDisableTabIndex' to Dialog component #1506

Closed bobbylkchao closed 3 months ago

bobbylkchao commented 3 months ago

Desc:

When I fixing a Penny chat bot accessibility issue, we couldn't swtich elemenets in chat bot modal by using 'tab' key. After a long investigation, I found it's due to tabindex='-1' in <Dialog/> component. When I remove tabindex attribution from <Dialog/>, the accessibility issue is gone, we can swtich elemenets in chat bot modal by using 'tab' key. And I also found the tabindex='-1' in Dialog elemenet seems it's a default value. (Video recording of this issue https://app.screencastify.com/v3/watch/L7vnzLf7ZWQYuIibKmsh)

So I have two options to fix it,

Option 1 is to write a logic in penny chat bot, to get rid of tabindex from <Dialog/> when modal is rendered.

Option 2 is to update <Dialog/> component in pcln-design-system, to accept a new optional parameter, eg. shouldDisableTabIndex, if this param is true, then not add tabindex to element.

My personally I perfer Option 2 much more.

Beta version:

6.17.1-beta-bobby-1

Testings screenshots:

image

image

image

image

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.78%. Comparing base (1bc5d81) to head (da403e2). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1506 +/- ## ========================================== + Coverage 94.67% 94.78% +0.11% ========================================== Files 146 146 Lines 11036 11039 +3 Branches 691 686 -5 ========================================== + Hits 10448 10463 +15 + Misses 565 558 -7 + Partials 23 18 -5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.