-
Notifications
You must be signed in to change notification settings - Fork 6
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
Popover improvements #730
Popover improvements #730
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://icy-water-049ec4003-730.westeurope.3.azurestaticapps.net |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: Has probably been a problem before your changes, but the trigger always needs to be able to activate the popover via keyboard controls. I would suggest to additionally trigger the popover via the focus event when triggerOnHover is true.
What if we want to implement keyboard support in a different way? should I add a prop that disables keyboard support? |
What do you mean with "different way"? From an accessibility perspective focus would be the expected behavior, I think. You could, of course, always implement additional keyboard controls. Opening on click instead of focus, even when triggerOnHover is true, would work as well. |
We show a separate arrow button that opens the popover. So basically there is one popover trigger for on hover and one for on click for keyboard support. Having both open via keyboard would break the behaviour because a single click selects the default thumbs up emoji instead of opening the popover. |
I see. Then a prop to disable the keyboard controls should be fine. |
okay, added. |
Azure Static Web Apps: Your stage site is ready! Visit it here: https://icy-water-049ec4003-730.westeurope.3.azurestaticapps.net |
Summary
Review Checklist
General
For new or updated components
https://swirl-storybook.flip-app.dev/?path=/docs/contributions-merge-publish--page