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

Add media hover variant #7939

Closed
wants to merge 1 commit into from
Closed

Conversation

browner12
Copy link
Contributor

This allows us to target devices that have a primary input device with hover capabilities.

The naming probably needs a little discussion, and I'm not sure how to test exactly, but wanted to get the conversation started.

It might also be of value to discuss having a configuration option to set a default for all hover classes throughout the site.

This allows us to target devices that have a primary input device with hover capabilities.

- https://developer.mozilla.org/en-US/docs/Web/CSS/@media/hover
- https://caniuse.com/mdn-css_at-rules_media_hover
@adamwathan
Copy link
Member

Definitely think it's worth adding this but the API is tough — hover-hover:... is just horrible 😂 I don't really have any obviously better ideas either though 😕

Couple random ideas (none of which I'm sold on at all):

  • @hover and @hover-none, trying to signal the @media bit
  • can-hover and no-hover

The other challenge is making sure the API leaves room for any-hover support if we wanted to add that as well:

https://developer.mozilla.org/en-US/docs/Web/CSS/@media/any-hover

We've discussed making it just the default like you mentioned, but hesitation around that is that a lot of people do things like show/hide menus on hover and if this were the default, those menus would become impossible to activate on mobile. Making it an option isn't a bad idea but I don't know where the option would go or what it would be called, as we don't have any options like that currently.

One way to do that in user-land already though is to override the built-in hover variant with your own variant:

https://play.tailwindcss.com/2jkPwSvj1p?file=config

module.exports = {
  theme: {
    extend: {
      // ...
    },
  },
  plugins: [
    function({ addVariant }) {
      addVariant('hover', '@media (hover: hover) { &:hover }')
    }
  ],
}

Will leave this open to discuss for now — hopefully can come up with something that feels good 👍

@browner12
Copy link
Contributor Author

I'm curious, for people relying on a hover event to show/hide menus on mobile, aren't they running a huge risk by assuming the specific mobile browser sends a hover event?

@adamwathan
Copy link
Member

I'm curious, for people relying on a hover event to show/hide menus on mobile, aren't they running a huge risk by assuming the specific mobile browser sends a hover event?

Yeah it’s a bad thing to do for sure, but it would be a breaking change for us because some people are doing it without realizing the issues with it.

I think ideally this would all just be the default though, maybe can figure out a way to do it that’s low impact.

@browner12
Copy link
Contributor Author

I remember in the v1 --> v2 upgrade, we had some "future" and "experimental" flags.

https://v2.tailwindcss.com/docs/upgrading-to-v2#remove-future-and-experimental-configuration-options

Do these options still exist, and would this be a good path for moving this to the default?

@MichaelAllenWarner
Copy link
Contributor

This seems like a great idea.

Re @adamwathan's suggestion for overriding Tailwind's built-in hover: variant, I've started doing it like this:

module.exports = {
  theme: {
    extend: {
      // ...
    },
  },
  plugins: [
    function({ addVariant }) {
      addVariant(
        'hover',
        '@media (hover: hover) and (pointer: fine) { &:hover }'
      );
      addVariant(
        'group-hover',
        '@media (hover: hover) and (pointer: fine) { :merge(.group):hover & }'
      );
      addVariant(
        'peer-hover',
        '@media (hover: hover) and (pointer: fine) { :merge(.peer):hover ~ & }'
      );
    }
  ],
}

Without pointer: fine you'll get undesired behavior on at least Chrome for Android. Plus, you probably want those group- and peer- variants, too.

@adamwathan
Copy link
Member

Going to be working on this a bit soon to come up with a plan but wanted to share this that I stumbled across which could be helpful:

https://www.npmjs.com/package/postcss-hover-media-feature

@adamwathan
Copy link
Member

@browner12 Landed on this approach: #8394

Let me know what you think in the comments there if you have any thoughts, but going to close this one (included you as co-author on the other one 👊 ).

We might still want to add explicit variants for just the media portion (like hover-none:poop and hover-hover:poop, or maybe can-hover:... and no-hover:...) but let's treat that as a separate conversation, and only add them by default if we really think there's practical value to it. I try to avoid having to come up with good names and pour concrete on API decisions whenever we can avoid it!

@adamwathan adamwathan closed this May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants