fix: keyboard interaction for popover

[jacobdevera]

Description

• focus the first element in the popover when opened, and traps focus within popover until closed with Escape key
• allows other elements in the popover to be interacted with (e.g. buttons, inputs, other popovers)
• returns focus to the element that opened the popover when closed with Escape key

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[netlify]

Deploy preview for fundamental-react ready!

Built with commit bc64b61

https://deploy-preview-793--fundamental-react.netlify.com

[jacobdevera]

innerRef could not be set without the props going to this level because it only exists in react-popper.

[jbadan]

We might need an additional prop then to spread to the inner div

[jacobdevera]

I decided to just separate the ref out from the popperProps. Don't think there would be a need to have two sets of props to spread.

[meganaconley]

The combobox can't be navigated into with the arrow key. Is this due to keyboard issues of the list inside or can we fix this here?
If it's due to the list component let's make a follow up task to fix that.

[meganaconley]

Looks good, QA completed and now the keyboard handling is up to spec.

[ChristianKienle]

I have a question – and a comment. :)

Fundamental Vue also has a Popover-component. I also thought about where to add things like keyboard-handling, trapping, … and in the end (at least as of now) I decided to not implement those things in the Popover component itself.

Those kind of things are implemented in the components that use the popover component.

The reason was the realization that this makes it more straight forward to re-use the popover component for a multitude of other things. For example I am also using the popover for popovers that are shown "on hover" and in that case I don't want any focus trapping to happen – also ESC should not be captured/handled by the popover that just acts as a tooltip…

What are your thoughts on this? Maybe I am also over-engineering things… :D

[jacobdevera]

@ChristianKienle Good points. 🙂 I think this boils down to what a Popover component really is. Searching around, it seems many component libraries make a distinct differentiation—a Popover is explicitly triggered by click/key press; whereas a Tooltip strictly appears on mouse hover/keyboard focus.
e.g. Bootstrap:
https://getbootstrap.com/docs/4.4/components/popovers/
https://getbootstrap.com/docs/4.4/components/tooltips/

But this isn't consistent, as react-bootstrap uses tooltips that need to be activated as well, with the main differentiation from its popover being the styling.

We don't really have a Tooltip equivalent in Fundamental, but the majority of the keyboard handling is in the separate focusManager class, besides the escape key handling from what I can see. Perhaps keyboard handling could be made optional via a prop to make it act as a tooltip, but I'm not sure how else I'd compose the Popover differently. To me, it seems the base popper is the true abstraction, and Popover is just an implementation of it.

[ChristianKienle]

@jacobdevera Take the date-input + calendar component as an example.

In Fundamental Vue this component is simply using the popover component + a date input component + the calendar. Once you click on the input the calendar appears. While the calendar is open you still want to be able to use the input (typing in a data + using the arrow keys + esc). Isn't that a use case where you would have to "turn off" almost every special keyboard-handling + focus trapping code?

The popover in Fundamental Vue is really just a div that is rendered on top of everything else + managed by popper.js.

There are special components that simply re-use the popover component but add "domain specific" things like keyboard handling, focus trapping + auto dismiss when the user interacts with elements outside of the popover.

The popover component in Fundamental Vue does not even "dismiss itself" when the user starts to interact with something outside of it. One of the reasons is the above date picker. You want the user to interact with the input element while the popover is open. :D When I realized that I first began to add props to the (back then – much smarter) popover component that could be used to toggle those features on and off. In the end I almost always had to play with those props because every time I tried to re-use the smart popover something did not feel right.

This was the point when I said to myself: "Hey – I am turning off features every time I use this component – why don't I just remove all those features then and add them later (in other components that are part of the library) where they belong to?. Then I made that change and it felt good. But I guess you are right – there is no "right" and "wrong" and definitive way to do things here. :D

Also I should mention that one part of me feels my approach is way too over engineered – it somehow makes sense in my head but is not really intuitive because you have other expectations when you hear "popover".