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

[EuiToolTip] Close tooltips on escape keydown #7751

Merged
merged 2 commits into from
May 14, 2024

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented May 13, 2024

Summary

closes #7741

tooltip_escape

QA

  • Go to https://eui.elastic.co/pr_7751/#/display/tooltip
  • Keyboard tab to the first tooltip link
  • Press the Escape key and confirm that the tooltip is dismissed/hidden
  • (regression testing) With the tooltip trigger still focused, move your mouse cursor over it and confirm the tooltip correctly appears and disappears as usual on hover/mouseout

General checklist

  • Browser QA
    - [ ] Checked in both light and dark modes
    - [ ] Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA - Skipped docs - see below conversation with Trevor
    - [ ] Added documentation
    - [ ] Props have proper autodocs (using @default if default values are missing) and playground toggles
    - [ ] Checked Code Sandbox works for any docs examples
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist - N/A

@cee-chen
Copy link
Member Author

@1Copenut do you think this interaction needs to be documented in our docs, or that we need extra screen reader text explaining it?

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

@1Copenut
Copy link
Contributor

@1Copenut do you think this interaction needs to be documented in our docs, or that we need extra screen reader text explaining it?

I think what we have for now is just enough. The benefit will be for keyboard users primarily. It's a pretty common pattern to use ESC to close, so I feel it'll be a quick self-discover.

@cee-chen
Copy link
Member Author

Sounds great, thanks!

@cee-chen cee-chen marked this pull request as ready for review May 13, 2024 21:51
@cee-chen cee-chen requested a review from a team as a code owner May 13, 2024 21:51
Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🐈‍⬛ Looks good to me and works as expected 👍

@cee-chen
Copy link
Member Author

Thanks y'all!

@cee-chen cee-chen merged commit 4d795a5 into elastic:main May 14, 2024
8 checks passed
@cee-chen cee-chen deleted the tooltip/esc-to-close branch May 14, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EuiTooltip][KEYBOARD]: Would we consider adding ESC keypress to dismiss the tooltip?
5 participants