[EuiFlyout] Clarify clickOutsideDisables decision

[thompsongl]

Summary

#4071 changed the default config of the focus trap in EuiFlyout to clickOutsideDisables=false. This altered behavior slightly:

It does change the existing behaviour where opening a flyout without ownFocus would reset focus to the button when clicking out of the flyout, instead of selecting the button again when the flyout closes. This is a change in behaviour but feels much better to me.

What we see now is that there is no way to exit the focus trap (except for clicking the close 'X') when ownFocus=false. For instance, in the "Small flyout, without ownFocus" example, opening the flyout and attempting click the global search input results unexpectedly in the input not gaining focus. This seems odd considering that there is no overlay mask to indicate that elements "under" the flyout are not focusable.

I see two options:

1. (currently committed in this branch) Change clickOutsideDisables={false} to clickOutsideDisables={!ownFocus}
   The return-focus bug is still prevented for ownFocus=true and more intuitive behavior returns for ownFocus=false.
2. Keep clickOutsideDisables={false}, but indicate through documentation that consumers need to add their own logic to close the flyout on outside clicks. This would essentially codify that having an open flyout and expecting to interact with elements outside of the flyout is not supported.

This also has implications for #2224, which can be closed by a decision either way (it shows a case of ownFocus=false while attempting outside interaction).

Note: #4071 has not been released yet, so you need to use local master or the docs preivew in this PR

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in Chrome, Safari, Edge, and Firefox
- [ ] Props have proper autodocs

• Added documentation
• Checked Code Sandbox works for the any docs examples

- [ ] Added or updated jest tests

• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[kibanamachine]

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

[cchaos]

Ok so from what I can tell the new behavior is:

ownFocus = true

1. Shows overlay mask (clicking closes flyout)
2. Traps keyboard focus
3. On close, returns focus to originating button

ownFocus = false (default)

1. Doesn't show overlay mask
2. Traps keyboard focus
3. Allows user to interact with other parts of the underlying page (including opening more flyouts - not new behavior and not sure we can do anything about it)
4. On close, returns focus to originating button

I think the big question is, is no. 2 in the ownFocus = false list ok. It means that keyboard-only users can never interact with the underlying page while a flyout is open. IMO, I think it is ok. I think for keyboard users it's more of a nuisance if they lost focus of the flyout but need to close it. They'd have to hunt it down.

The other thing I would do in this PR is to change any of the flyout examples that use ownFocus = false to ensure the triggering button actually toggles the visibility. Similar to how we handle popovers.

[thompsongl]

I think the big question is, is no. 2 in the ownFocus = false list ok. It means that keyboard-only users can never interact with the underlying page while a flyout is open. IMO, I think it is ok

I agree with this. And this behavior existed before this PR and before #4071, anyway.

This PR (as it stands now) really only has an impact on non-keyboard users who attempt interact with focusable elements outside the flyout. It prevents cases where their clicks might appear to be blocked/prevented, but with no indication of why.

[chandlerprall]

The small flyout example states

... [ownFocus=false] not only removes the focus trap around the flyout ...

But it does still trap focus. Either the documentation needs to be changed (aligns with this PR), or the focus trap needs to be disabled (add disabled={!ownFocus}) instead. I slightly lean toward keeping the focus trap while allowing interaction with other parts of the page. @cchaos thoughts?

[cchaos]

Yes, I would update the example by just removing that bit about the prop also removing focus trap.

[thompsongl]

Great, I'll update the example and add some code comments

[kibanamachine]

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

[chandlerprall]

The ownFocus prop's description should be updated to only reference the removal of the overlay mask.

[kibanamachine]

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

[kibanamachine]

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

[cchaos]

👍 Thank you!

[chandlerprall]

⛵ 🦑

[thompsongl]

Going to wait to see how #4247 is resolved before merging this. Should be safe, but want to verify

[kibanamachine]

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