Responsive tooltips #767
Responsive tooltips #767
Conversation
There was a problem hiding this comment.
Is this a breaking change? Kinda... but it could also be considered fixing a bug on mobile (patch). And adding a new modifier class (minor). Anyways, when shipping this on .com, we would have to add .tooltipped-touch to all these "click icon to get more information" for users that are on a touch device and visit the "Desktop version".
| // | ||
| // Hide tooltips when primary input mechanism cannot hover (default) | ||
| // Enable tooltips for touch input with .tooltipped-touch | ||
| @media (hover: none) { |
There was a problem hiding this comment.
There is also the any-hover media query which might be useful for these hybrid devices (laptops that have a touch screen and mouse). But I don't have any device to test that. 😉 Also not sure if "One or more available input mechanism" is that helpful, instead it should be "the currently in use input method" so that you can switch between a mouse and touch screen whenever. 🤔
| // Hide tooltips when primary input mechanism cannot hover (default) | ||
| // Enable tooltips for touch input with .tooltipped-touch | ||
| @media (hover: none) { | ||
| .tooltipped:not(.tooltipped-touch):hover { |
There was a problem hiding this comment.
Is .tooltipped-touch a good modifier class name? Or should it be more descriptive .tooltipped-show-on-touch .tooltipped-force-hover???
|
I pulled this change into https://github.com/github/github/pull/113518 for a bug fix |
|
I'm still not too sure about the |
What's the status of this now that github/github#113518 is merged? |
We haven't really made a final decision. I think one problem with relying on I think the better options moving forward is:
|
|
👍🏻 on avoiding hover. Seems like a pain for accessibility and touch screen users. Maybe we can file a ADR? |
|
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
This makes the tooltips "responsive". More specifically:
.tooltipped-touchmodifier class.For example the
infoicon does nothing except showing more information with a tooltip. In this case it would be ok to use.tooltipped-touchso that you can tap on it also on a phone/tablet to make the tooltip appear.Tested on
Concerns
We have been testing part of this with https://github.com/github/github/pull/113518. A problem reported by a user is that:
is currently broken. The browser thinks that the "primary" input device is touch so hovering with the mouse does not work. An alternative might be to use
any-hover:noneinstead ofhover:none. But that would probably mean that when the user would use the touchscreen, the hover effects are not disabled because the media query only reacts to "what input device is available" and not "what input device is currently in use"./cc @primer/ds-core
Issue https://github.com/github/design-systems/issues/558
Test on .com: https://github.com/github/github/pull/113518