eui
eui copied to clipboard
elastic
[EuiSelectable] add support for option tooltips
Summary
This is part 2 of #7690 (see part 1 here) closes #7690
This PR updates EuiSelectable to support the two props toolTipContent and toolTipProps to be passed to EuiSelectable options to display a tooltip on hover/focus.
To ensure the tooltip can be opened on focus, the EuiToolTip was updated to a support controlled behavior via isOpen. (this change is currently copied here from the previous PR for part 1)
QA
- [ ] review and test EuiSelectable in storybook and docs and/or checkout this PR locally via
gh pr checkout 7715- [ ] ensure the tooltip is applied as expected (on hover as well as on keyboard navigation or click)
General checklist
- Browser QA
- [x] Checked in both light and dark modes
- [x] Checked in mobile
- [x] Checked in Chrome, Safari, Edge, and Firefox
- [x] Checked for accessibility including keyboard-only and screenreader modes
- Docs site QA
- [x] Added documentation
- [x] Props have proper autodocs (using
@defaultif default values are missing) and playground toggles - [ ] ~Checked Code Sandbox works for any docs examples~
- Code quality checklist
- Release checklist
- [x] A changelog entry exists and is marked appropriately.
- [ ] ~If applicable, added the breaking change issue label (and filled out the breaking change checklist)~
🗒️ I added VRT snapshots to this PR but the initial state of the tooltip story is equal to the playground one. We should add interactions to create a different state for Loki to snapshot, but I'd suggest to do it in a separate PR to keep things scoped. WDYT?
Hey Lene! I'm so sorry for not reviewing this in time for Monday's release - I'm leaning towards shipping with just the combobox tooltips for this week and getting selectable tooltips in next week if that's okay!
Hey Lene! I'm so sorry for not reviewing this in time for Monday's release - I'm leaning towards shipping with just the combobox tooltips for this week and getting selectable tooltips in next week if that's okay!
Of course, no worries - that's no problem! 🙂
@1Copenut I'll probably tag you in to test the screen reader experience of this tomorrow. It's working great for me on Firefox+VO but buggy for Safari+VO 😭 (it reads the tooltip description after moving away from the item on keyboard arrow navigation press, which is all kinds of weird)
@cee-chen Thanks for the changes! The updates look good to me 👍 I'm fine keeping the function component conversion in this PR - it's not part of the scope but it's a nice update anyway. One more step towards "no more class components" 🌈
Arggh. Thank you so much for confirming my findings Trevor, I was seeing that same behavior in Safari+VO as well.
Oh actually YOU KNOW WHAT. The baseline EuiSelectable component is doing this all by itself even without tooltips, what the heck. I can repro it in the production EUI docs right now: https://eui.elastic.co/#/forms/selectable
You can see it repeating the old/previous item that was navigated away from before it announces the next item, and it doesn't seem to announce the full current/next item or its state until after.
@1Copenut I think we need to file an issue for this separate from this PR, as something funky is definitely going on here even outside of tooltips.
I'm going to go ahead and merge this PR for now since the issue is with Safari/VO, but we should probably look into a fix separately 😬
Preview staging links for this PR:
- Docs site: https://eui.elastic.co/pr_7715/
- Storybook: https://eui.elastic.co/pr_7715/storybook
:green_heart: Build Succeeded
- Buildkite Build
- Commit: bcf9e6d12f2d04e92593e9daddee596de66cfa77
History
- :green_heart: Build #1908 succeeded 48515874d2229171bbd98f882a5f94c28546cbd0
- :green_heart: Build #1890 succeeded 196c785b5df010175b25db98c418a9940b5fc1d9
- :green_heart: Build #1857 succeeded c5d30a4c13a4c0ab659e28e4afce71abbaa2e38a
- :green_heart: Build #1839 succeeded 4df1f0b3333e6bcc3b94192b7e867f48a99c2014
- :broken_heart: Build #1831 failed 1929ac44a1d8707fe867d8884649c1afa3fb68af
