Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a floating close button to all "popovers" #3024

Closed
myasonik opened this issue Mar 10, 2020 · 1 comment
Closed

Add a floating close button to all "popovers" #3024

myasonik opened this issue Mar 10, 2020 · 1 comment

Comments

@myasonik
Copy link
Contributor

myasonik commented Mar 10, 2020

Preamble

Before digging into this, I just want to define "popover" as I mean it in the title. I don't mean EuiPopover and I also don't mean anything that has a z-index that floats it above content. By popover, in this case, I mean anything that can be focused or focused within that is displayed on top of other content. (Some examples are Modal, Flyout, Popover, ContextMenu)

The issue

So I'll break this up into two parts just for discussion:

  1. For popovers that trap focus, some of the examples already provide a close button but others don't. Most of the examples allow you to turn off the close button and expect the implementing developer to add one.

    For screen reader users, they get a hidden message telling them that they can use ESC to close it but they're actually the only group that is ever told this. Mouse users have to guess/hope/mash that clicking outside the modal will close it (which, maybe isn't a huge leap but still) and keyboard users have to do the same with ESC (which, again, maybe isn't a huge leap but given many touch-bar Macs, I'd prefer to not be overly reliant on that).

  2. For popovers that don't trap focus and let you tab through them (I think Popover allows this but I could be mistaken), it's basically the same issues as above but at least the user can still use the rest of the page so it's not as bad.

Prior art

In #2977 @cchaos introduced the first use of a floating close button for the CollapsibleNav. I think something exactly like this could work for many of our popovers.

If the popover creates some sort of mask on the content underneath, what was introduced there works perfectly. If there's no mask, the floating button will need a bit more design treatment. Just some sort of backplate so the text is still visible no matter where the popover is rendered.

An alternate idea

  • Kind of splitting the difference on all this, taking advantage of the recently merged showOnFocus prop for ScreenReaderOnly, we could show the floating button only on focus helping out keyboard users without making much of an impact on the design.
  • Also, just wanted to add for popovers that we know show a close button, we wouldn't need a secondary floating one (though we could migrate all to this design if we liked it). So, for example, Modal wouldn't need one and a Flyout with hideCloseButton set to false (the default) wouldn't need one either.
@cchaos
Copy link
Contributor

cchaos commented Sep 19, 2020

I've added a link to this in a line item on #4040. Going to close this one so we have all the discussion in one place.

@cchaos cchaos closed this as completed Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants