[EuiFlyoutBody] Add scrollableTabIndex prop

[cee-chen]

Summary

This PR is part of #7041 (several a11y improvements/enhancements for the new EuiCollapsibleNavBeta work).

This PR only addresses the extra tab stop caused by EuiFlyoutBody - it does not address any issues inherent to push flyouts (#6576). I'll be looking into that as a separate PR.

QA

• gh pr checkout 7061
• yarn storybook
• Go to http://localhost:6006/?path=/story/euicollapsiblenavbeta--kibana-example
• Click the collapse nav button and press tab - there should only be 1 tab stop (instead of 2) between the toggle button and the flyout content
• Run axe devtools, there should be 0 issues caught

General checklist

• Props have proper autodocs (using @default if default values are missing) and playground toggles
  - [ ] Added documentation - Opting for just props documentation, I don't think this needs its own docs example
• Added or updated jest and cypress tests
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[cee-chen]

@1Copenut I know you're busy with other work this week (Kibana upgrade, buildkite CI) so I'm mostly just tagging you for info. If you see any issues with this implementation or with the docs, please feel to shout.

@breehall I'm hoping you can grab the main review/QA of this PR, since you've had some experience QAing this component and noticed the focus/tab UX previosuly.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_7061_buildkite/

[cee-chen]

buildkite test this

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_7061_buildkite/

[cee-chen]

buildkite test this

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_7061_buildkite/

[breehall]

This is a great accessibility improvement. When tabbing in CollapsibleNavBeta in Storybook, there's only one tab stop as expected and there are no Axe errors on the page.

[Question]
What do you think about adding a small blurb to this in our docs? This is really great info and could be very useful for devs to know.

[Follow up]
This prop also the following examples for EuiFlyout in our docs. The same behavior is occurring where there is an actionable and focusable item (like a button or button group) and there's a skip in focus. I applied the new prop locally and it fixes them - I can create a separate docs PR to update these!

[cee-chen]

I would strongly rather not update our EuiFlyout docs examples with this prop. As the prop documentation specifies, unsetting tabIndex={0} is only optional if you know your flyout children has focusable content. In the case of EuiCollapsibleNav, we can be pretty damn sure it'll contain focusable children, since it's intended to contain links. So keyboard scrolling is not an issue for this specific usage of EuiFlyoutBody.

In the case of other flyouts - we have absolutely no idea whether that's true or not. Consumers could have an entire essay in there of just text without any interactive children, which means that EuiFlyoutBody needs to have tabIndex={0} in order for sighted keyboard users to be able to scroll the region with arrow keys.

As we've mentioned before in the past, consumers tend to blindly copy and paste what's in our docs examples without giving too much thought of whether they need to change it or not. In those scenarios, we need to default the scrollable region having a tabIndex. The a11y experience of a user not being able to scroll via keyboard is breaking vs having an extra tab stop (a mild annoyance) - we should absolutely not be defaulting to the "breaking" experience.

[cee-chen]

What do you think about adding a small blurb to this in our docs? This is really great info and could be very useful for devs to know.

Personally, I think the documentation added for the prop fully covers the information needed, and like I mentioned above, I'd rather not have consumers who have no idea what they're doing (in terms of a11y) customize this prop. It's there if devs know enough to even look for it, otherwise I'd rather have it working as a sensible default in the background without devs needing to configure it unless they know what they're doing.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_7061_buildkite/

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 06a77ac

History

• 💚 Build #263 succeeded fd3b379
• 💔 Build #261 failed fd3b379
• 💔 Build #258 failed fd3b379

[breehall]

Thanks for your thorough explanation. To confirm, we won't be moving forward with updating our flyout docs or creating additional documentation

[cee-chen]

Yep! Thanks Bree. Going to go ahead and merge, but Trevor, feel free to chime in or follow up on Slack if you disagree - we can circle back to this later if needed.