Popover: wrong tab order for popover contents

[vindl]

See p7jreA-1wH-p2 for related discussion.

Steps to reproduce

1. Open up any page that uses Popover component (see instructions here for example https://github.com/Automattic/wp-calypso/issues/21659)
2. Use keyboard only to navigate to Popover and open it.
3. Press Tab key to focus Popover content.

What I expected

Items inside Popover to be focused first.

What happened instead

Items in the background were focused, and Popover content comes in last in tab ordering.

Please refer to https://github.com/Automattic/wp-calypso/issues/21659 for one special case of this issue.

[ryelle]

I decided to dive into this for a hack week project, and wow it's complicated. I tried two approaches, neither worked well so I'm not submitting a PR 🙁 You can see the first attempt using react-modal, and the second attempt, injecting the popover into the correct place in the source order. The problem with react-modal was more to do with the various ways we use popovers (+ a scrolling bug), while the trouble with the second approach was getting the position-detecting logic to work consistently.

So maybe we should try something different. We have a few different kinds of pop-up content, which all use Popover as a base.

1. The tooltip/InfoPopover use: this is purely information - like the plans page ℹ️ icon, which describes each plan feature
2. The Popover with actions: this needs some kind of user interaction - like the card checkout popover, which has the "Checkout" button, I would also put the author switcher, site popover, etc in this bucket
3. The PopoverMenu/EllipsisMenu/SplitButton/etc: A list of actions or links - like the actions on a page or post
4. The ephemeral tooltip, which only displays on hover - this is the Tooltip component, which uses Popover but styles it differently, and generally uses mouseenter/mouseleave events to display.

I think trying to use the same base component for all 4 of these cases is not the right approach. Each has slightly different interaction patterns, which is more obvious if you try to map it to ARIA interaction patterns. react-modal sets the popover container to the dialog role, and along with the focus trap + an aria-hidden attribute, makes it so that keyboard & screen reader users can't interact with the page while the popover is open. But dialogs are meant to have a focusable element inside them, some action to take. Which makes sense in case 2, but not 1. For case 1, we want something more like the "Toggletips" pattern described here.

Additionally, the PopoverMenu is using the menu/menuitem roles, which is fine for action buttons, but does get confusing if added to a link (more info). This component in particular I think should be split from the Popover, and instead of injecting an absolutely positioned item into RootChild should be put inline with the rest of the HTML. This way we can keep the tab order correct without having to worry about managing focus locations.

Finally are Tooltips – generally these are not accessible at all via keyboard, since they're only available on hover. I think the approach in "tooltips as auxiliary info" would work, but would require only text content in the tooltips (which we don't enforce, but is probably the case? the most complex tooltips seem to be on stats). This would also separate out Tooltip from the base Popover.

Given all the above, I think the approach to making our popovers accessible should be to move away from one parent Popover component as much as possible:

• Update Tooltip to not use Popover, make the parent element focusable, show it on focus as well as hover, and add the aria-describedby code to announce it for screen readers
• Update PopoverMenu to not use Popover, instead inlining the menu to preserve tab order
• Update InfoPopover to not use Popover, instead to follow the "toggletips" example linked above
• Finally we can revisit Popover, which should only be used for "complex" pop-up content, like the checkout screen, or comment user info, and could be changed to a dialog using react-modal.

That said, this is definitely a larger project than 1 person's week - I'd be happy to team up with a few interested people to talk it out more & maybe try the approach I outlined above.

[aduth]

Accessible popovers have been the source of much distress for me in the past, so I'd love to help in any way I can to avoid similar headaches in your pending adventure 😄

I'd point to https://github.com/WordPress/gutenberg/pull/5595 as an example of my own previous attempts and failings in Gutenberg, which was eventually closed after some embattled back-and-forths. Specifically, I might hope that my concluding overview at https://github.com/WordPress/gutenberg/pull/5595#issuecomment-380488105 could serve as a useful reference, as I'm certain many of the components which exist in Gutenberg have similar parallels in Calypso as well. I actually came to a different conclusion: Noting that a base Popover component can be a good thing, but not in the sense of advanced usage we've come to expect from it (i.e. stripping it of most of its current behaviors).

This component in particular I think should be split from the Popover, and instead of injecting an absolutely positioned item into RootChild should be put inline with the rest of the HTML. This way we can keep the tab order correct without having to worry about managing focus locations.

On the accessibility front, this certainly does make things simpler. But see my comment at https://github.com/WordPress/gutenberg/pull/5595#issuecomment-375153865 for some cautions to be aware of:

And it's not a matter of the other bugs being the more easily addressed issues than recreating accessible behaviors via slotted rendering. There's simply no known solution to issues affecting stacking context with relation to popovers being shown above other elements on the page (and any such solution may require entire refactors of the administrative markup and CSS, potentially breaking plugins in the process) and unavoidable future bugs and maintenance headaches caused by inadvertent cascade of CSS. Even beyond this, it still wouldn't just work with keyboard behaviors because, for performance reasons, the popover content isn't within the DOM until it's expected to be shown, to avoid rendering and reconciling the potential thousands of nodes that could be made visible at any given point in time.

[stale[bot]]

This issue has been marked as stale and will be closed in seven days. This happened because:

• It has been inactive in the past 9 months.
• It isn't a project or a milestone, and hasn’t been labeled `[Pri] Blocker`, `[Pri] High`, `[Status] Keep Open`, or `OSS Citizen`.

You can keep the issue open by adding a comment. If you do, please provide additional context and explain why you’d like it to remain open. You can also close the issue yourself — if you do, please add a brief explanation.

[sirreal]

🙅‍♂️ Keep it open, stalebot.

[diegohaz]

I was taking notes on accessibility issues I was finding while working on other stuff, and I just encountered this issue with Popover.

I think we can say that a keyboard user can't access the content of the popover as when they reach it at the end of the document it's completely out of context.

I'd like to suggest taking a look at this Popover implementation, which handles modal and non-modal states (modal prop), blocking body scroll (preventBodyScroll prop) and conditionally rendered content.

[github-actions[bot]]

This issue is stale because it has been 180 days with no activity. You can keep the issue open by adding a comment. If you do, please provide additional context and explain why you’d like it to remain open. You can also close the issue yourself — if you do, please add a brief explanation and apply one of relevant issue close labels.

[cuemarie]

👋 Hey folks! Since this issue has been inactive for quite some time, KitKat has made the decision to close it.

• Internal reference, for more on this decision: pdqkMK-14B-p2

If you think this issue warrants another look, here are some next steps!

1. Report anew: A new report with more current details and steps to replicate may be the best way to renew attention on this issue. Feel free to refer back to this closed issue in your report!
2. Reopen: If you feel the issue still matches the context/history here, you can also reopen the issue and add fresh logs, screenshots and steps to reproduce.

Thanks for your involvement!

📌 ADDITIONAL NOTES Current efforts to audit a11y issues will include filing new issues for popover problems as they're found, so if this issue persists, it'll be re-reported with updated details in the future!