reactjs / react-modal

Accessible modal dialog component for React
http://reactcommunity.org/react-modal
MIT License
7.37k stars 809 forks source link

Focus escapes modal when radio buttons in modal #636

Open WickyNilliams opened 6 years ago

WickyNilliams commented 6 years ago

Summary:

When there are radio buttons in a modal, you can escape focus with shift-tab.

Steps to reproduce:

Steps are included in the test case below

Expected behavior:

You should not be able to escape the modal's focus

Link to example of issue:

https://codesandbox.io/s/my9vm34m68

Additional notes:

Not sure if it's related specifically to radio buttons, but that is what is causing the issue for me

WickyNilliams commented 6 years ago

I think the issue here is that radio buttons are considered individually focusable by findTabbable(), but the browser treats a group of radio as a single focusable item when tabbing, if their name attribute matches.

WickyNilliams commented 6 years ago

I've also noticed that if you hold your mouse down on a radio button then focus is shifted back to .ReactModal__Content. When you release the mouse, focus is then returned to the input.

I'm not sure if this a related or separate issue.

ryansukale commented 5 years ago

We ran into a similar issue. Here are some details.

Our modal contains a bunch of fieldsets. Each fieldset has several radio buttons. In the footer of the modal is submit button that is disabled.

When you press tab on chrome, it only cycles through just the first radio button within each fieldset. Whereas on Safari, it goes to each radio button for each fieldset.

As a result, when we press tab to reach the first radio of the last fieldset, and then press tab again, it does not cycle to the modal close button because the first option of the last fieldset is not the tail element in the list of tabbable elements. Here's where the logic fails.

coulgreer commented 3 years ago

Hello, I don't mean to necro this, but has anyone found a workaround for this issue? I noticed that this issue is still open and I, too, am experiencing this issue :(

stephenmcm commented 1 year ago

🧟 Bringing this back form the dead has there been any work on finding a solution? The way this is currently handled looks to need enhancement to check for type=radio and then walk for all matching name fields.

The current solution is clearly a WCAG violation as it breaks navigation for keyboard users, at minimum the documentation should be updated to highlight this.

diasbruno commented 1 year ago

@stephenmcm @coulgreer @ryansukale The issue is still open. It's up for grabs - PRs are welcome.

doeg commented 8 months ago

Hey @diasbruno, I'm going to take another look into this one. 👀 Will update once I have more information!

diasbruno commented 8 months ago

@doeg Awesome. Let me know if you need anything.