Closed dpasque closed 1 year ago
@dpasque I love the new tooltips! The react-tooltip library was an excellent choice 👍
As a side fix, I added CSS to hide the tooltip icons if the screen doesn't support hovering (e.g. smartphones).
suggestion (blocking):
I actually think it would be beneficial to have the tooltip icons (for type and title) visible on mobile, but have the event change to click
instead. I don't know if having a different tooltip object for desktop vs mobile adds complexity, but I think it would be good to keep the helpful hints for all users. What do you think about that? The feature tree is harder and I'm leaning toward not having tooltips on mobile for simplicity.
Big questions: how does the styling feel? We can tweak if needed!
suggestion (blocking): Actually, the styling happens to be spot on from the Calypso tooltips, which makes things easy 😅 I did some quick UX research and it appears that there a few tweaks we could do to make it UX-friendly:
I experimented a bit and here's what I came up with:
<Tooltip
anchorSelect={ `#${ buttonId } span` }
delayShow={ 1000 }
className={ styles.tooltip }
content={ description }
place={ 'right' }
/>
I mostly just changed the anchor to be the text (span) instead of the entire row, removed the float
and noArrow
parameters and changed the placement to the right, so it doesn't overlap tree elements or other text. I did this on expandable-tree-node
, feature
, and type-title-form
and it seems to be working okay. I also changed the max width to 20rem
to keep the tooltip relatively compact. Let me know what you think!
I'm still wrapping up some accessibility testing, but I just wanted to send this to you in case you want to try it out!
@john-legg -- I LOVE how these look now with your new suggestions! 😍 Thank you for the detailed research and feedback! 😄 Just pushed up some changes -- want to see if they fit what you were expecting?
I LOVE how these look now with your new suggestions! 😍 Thank you for the detailed research and feedback! 😄 Just pushed up some changes -- want to see if they fit what you were expecting?
Woo! No problem 👍 I just finished accessibility testing and everything looks good. Your new changes are perfect, so I think this one is ready 🚀
Edit: I'd also like to mention that clicking the tooltip icons on the Type and Title screen feels a lot better on desktop now that you've added the click
functionality for mobile. The tooltip used to pop in and out erratically if you clicked on it (I guess it was interrupting the hover state or something). I like that it's nice and smooth now 😄
The tooltip used to pop in and out erratically if you clicked on it (I guess it was interrupting the hover state or something). I like that it's nice and smooth now 😄
@john-legg That one was really interesting! So, the actual underlying cause was that the icon was originally part of the label
. I had thought that might make a nicer click target for the inputs, but it actually just made things waaaaaay clunkier. Taking the icons out of the label
now feels so much better, I agree! 👏
Thanks again for all the help, your design input was pure gold here, again! 😁
What Does This PR Add/Change?
Context: pciE2j-1Pp-p2 -- usability testing changes.
This switches away from native browser tooltips (because they are too slow), to custom, more responsive tooltips!
Now...
As a side fix, I added CSS to hide the tooltip icons if the screen doesn't support hovering (e.g. smartphones). It felt like bad practice to have an icon you can't actually hover on!
Notes on Library Usage
I debated for a while about which library to use. I wanted something stable, but also really simple. These are just tooltips! I really wanted to avoid adding a ton of code and technical complexity to just make a tooltip appear more quickly.
In the end, React Tooltip seemed the best option:
Notes on Accessibility
I want to keep using our paradigm of making tooltip content visible to screen readers primarily through ARIA descriptions on the elements they describe. Unfortunately, the one thing this library doesn't make easy is ARIA hiding the entire tooltip. You can easily ARIA hide the contents, but the tooltip wrapper is still visible in the accessibility tree.
For now, I don't think we should worry about this. If you're using keyboard navigation and a screen reader, the tooltips never appear anyway! And if someone was using a mouse, hovering, and then trying to do screen reader navigation in that hover state (an extremely unlikely edge case), I figured it was better to just be more verbose and have it read the full tooltip rather than have it read an empty tooltip shell!
Testing Instructions
Big questions: how does the styling feel? We can tweak if needed!
Issues
Related to #
Closes #