theKashey / react-focus-lock

It is a trap! A lock for a Focus. 🔓
MIT License
1.27k stars 67 forks source link

Remove positive tabIndex #182

Closed crshmk closed 2 years ago

crshmk commented 2 years ago

A positive tabIndex has become a significant accessibility flag. The axe devtools bot flags any tabIndex={1} on your page as a serious error. Even though this element with a tabIndex of 1 isn't really part of the direct user experience, it's best to get rid of any positive indices on your page.

I found that when setting noFocusGuards, things worked fine when developing your storybook examples, but on my own setup this popped the focus out of the lock.

As I understand the issue, the tabIndex value of 1 here handles cases when devs have other elements on their pages with a positive index.

It seems to me there would be two options:

I believe the latter is a cleaner api and allows for default accessibility compliance, but the former offers backwards compatibility for devs who are using positive indices on their pages.

This PR has two commits for each option, respectively.

crshmk commented 2 years ago

Do you mind rerunning the build? Looks like a pipeline error.

TrevorRice commented 2 years ago

Hi all!

Is there anything I can do to assist with this improvement?

theKashey commented 2 years ago

usePositiveIndices(command/hook) -> hasPositiveIndices(situation specification)

The rest is unchanged. Thank you for your contribution.