react-overlays icon indicating copy to clipboard operation
react-overlays copied to clipboard

Accessibility - AutoFocus Modal or First Focusable

Open shank-eric opened this issue 6 years ago • 10 comments

As documented here, I'm seeing autoFocus set focus to the dialog element when the user opens a modal, so the code is functioning as designed, and I assume the primary reason for this feature was to address accessibility concerns.

However, I'm getting complaints from my accessibility testers about this and they're suggesting that the focus should go to the first focusable element in the modal when it opens. The most notable impact of the current functionality is it generally causes screen readers to read the entire contents of the modal instead of just the modal's title and the currently focused element.

I've been pointed to this spec and related example, as well as this blog post as support for this decision.

I'm fairly impartial personally, but I'd like to be able to simply use the default accessibility features from here. Are there any references I can use to support the current functionality when discussing it with my accessibility testers? Or are you open to changing the autoFocus functionality to focus the first focusable element instead of the dialog?

shank-eric avatar Mar 20 '19 16:03 shank-eric

We do want to focus the first input, I think – but it's easier on us to just focus the modal. I believe the user-visible effect outside of screen readers is the same.

taion avatar Mar 20 '19 16:03 taion

Right, this has only come up in the context of screen readers. The blog post I linked to originally includes a suggestion of how to find the first focusable element:

    this.dialogEl = dialogEl;
    
    var focusableEls = this.dialogEl.querySelectorAll('a[href], area[href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), button:not([disabled]), [tabindex="0"]');
    this.focusableEls = Array.prototype.slice.call(focusableEls);
    this.firstFocusableEl = focusableEls[0];
    this.firstFocusableEl.focus();

Alternatively, I found a simple library that works similarly: https://github.com/davidtheclark/tabbable.

Is this a change that you'd like to have then? I can start a PR if that would be helpful.

shank-eric avatar Mar 20 '19 17:03 shank-eric

cc @jquense who knows this better than me – what do you think of the logic above for modal autofocus?

taion avatar Mar 20 '19 18:03 taion

I don't think that's a good choice as a general behavior default, and not what native dialog's do either. If you want to focus the first input when the modal opens add autoFocus to it, the Modal will not move focus to itself if it's already inside the Modal

jquense avatar Mar 20 '19 18:03 jquense

It seems like it's generally suggested as best practice to make a modal accessible, but using the autoFocus on the first input works for this particular modal.

shank-eric avatar Mar 20 '19 19:03 shank-eric

I was curious about the native dialogs and am actually seeing that the spec seems to indicate that they should focus the first non-inert element inside of themselves when they're opened: "let control be the first non-inert descendant element of subject, in tree order."

I also found this example in MDN looks to be autofocusing the select control inside of it when it's opened without setting autofocus on the control.

Regardless of if you want to move forward with this, I greatly appreciate your time in responding and all you guys do in putting together an excellent library! 😁

shank-eric avatar Mar 21 '19 13:03 shank-eric

huh does seem like the behavior has changed, maybe just fixed. I'm always open to making our components more accessible, we do have to balance the relative cost to gain, and other peoples use cases as well tho. For instance i'd like to not have the autoFocus and enforceFocus props entirely since it's the correct behavior but folks always have niche use-cases or technical reasons it doesn't work.

We can spec out the minimal fix we can merge it in as the default. I'm skeptical that tabbable tho is the smallest implementation of this, it's a lot bigger than i'd expect from what should be a single selector

jquense avatar Mar 21 '19 13:03 jquense

Yeah, I was unsure of it myself, but I also wonder if there's edge cases that it handles beyond the snippet from the blog post above. For example, this is from the readme at tabbable:

If you're thinking, "Why not just use the right querySelectorAll?", you may be on to something ... but, as with most "just" statements, you're probably not. For example, a simple querySelectorAll approach will not figure out whether an element is hidden, and therefore not actually tabbable.

shank-eric avatar Mar 21 '19 13:03 shank-eric

o yeah i'm sure the code exists for good reasons, and covers all the edge cases. I don't think that we necessarily need to think about some of those edge cases. Case in point the focus finding logic for dropdown items is only .dropdown-item:not(:disabled) and that works well enough, even tho there are plenty of edge cases it doesn't cover

jquense avatar Mar 21 '19 14:03 jquense

Fair enough, I wasn't sure how encompassing we wanted to try to be with dealing with those edge-cases.

shank-eric avatar Mar 21 '19 14:03 shank-eric

I have same scenario . For me the first interactive element is focused and the Screen reader announces the first element but not announcing the title of the modal . I added both aria-labelledby and aria-describedby to the modal , it announces the title On Voiceover but not on Cromevox. Any suggestions?

abcd28 avatar Oct 07 '23 08:10 abcd28

The normal modal behavior is to focus the modal not the first interactive element. If it's focusing something else you may have autoFocus on an input set

jquense avatar Oct 10 '23 13:10 jquense