Only apply hover on devices that support hover #14500
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adamwathan
commented
Sep 23, 2024
Comment on lines
+450
to
+452
| variants.static('hover', (r) => { | ||
| r.nodes = [rule('&:hover', [rule('@media (hover: hover)', r.nodes)])] | ||
| }) |
There was a problem hiding this comment.
This is the only important change — dropped the loop because the signature of this one is different now and we never needed to store all this stuff in an array for any other reason anyways.
RobinMalfait
force-pushed
the
feat/hover-only-when-supported
branch
from
4fd60ee to
080bf6e
Compare
This allows us to just change the data attribute when making a change instead of relying on `hover:`. In this PR `hover:` will use `@media (hover: hover)` which is not implemented in Firefox on Linux at the time of writing this PR.
RobinMalfait
force-pushed
the
feat/hover-only-when-supported
branch
from
080bf6e to
f799577
Compare
This reverts commit f799577.
RobinMalfait
approved these changes
Sep 24, 2024
RobinMalfait
approved these changes
Sep 24, 2024
adamwathan
force-pushed
the
feat/hover-only-when-supported
branch
from
1aa7dd1 to
64c1efd
Compare
|
@adamwathan Curious why you decided not to include the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the
hovervariant to only apply when@media (hover: hover)matches..hover\:bg-black { &:hover { + @media (hover: hover) { background: black; + } } }This is technically a breaking change because you may have built your site in a way where some interactions depend on hover (like opening a dropdown menu), and were relying on the fact that tapping on mobile triggers hover.
To bring back the old hover behavior, users can override the
hovervariant in their CSS file back to the simpler implementation:I've opted to go with just
@media (hover: hover)for this because it seems like the best trade-off between the available options. Using(any-hover: hover)would mean users would get sticky hover states when tapping on an iPad if they have a mouse or trackpad connected, which feels wrong to me because in those cases touch is still likely the primary method of interaction.Sites built with this feature in mind will be treating hover styles as progressive enhancement, so it seems better to me that using an iPad with a mouse would not have hover styles, vs. having sticky hover styles in the same situation.
Of course users can always override this with whatever they want, so making this the default isn't locking anyone in to a particular choice.