-
Notifications
You must be signed in to change notification settings - Fork 22
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
Should we add a "Scoped Hover" mixin as a default #33
Comments
I think the approach makes perfectly sense for CSS module based approach 👍 . One suggestion is maybe to change the naming of the vars |
+1 for the mixin! |
@jesse-mm If we would set the default value to '.no-touchevents' that would mean that every time we use the this would result in something like this: @include scoped-hover('.link', '.no-touchevents .is-active') {
color: red;
} Personally i'm not a big fan of this! Renaming them to @ReneDrie we could leave out the no-touchevents by default and add a little bit of documentation at the top of the mixin on how to enable it? We do not want to add to much default configuration to a skeleton right? |
What if we move |
In "seng-scss" we have a "hover" mixin. But unfortunately this isn't working the way it's supposed to work from within the node-module.
The idea is to use a global class (most of the time a touch check from modernizr) as a prefix to the element. And then add :hover to the rule before displaying all content.
I think this is a nice solution to easily maintain hover states for supported devices only.
The problem with this is that we're working with CSS modules, where the local classes will interfere with the global classes. And also, by default, the modernizr isn't set-up to add the notouch class.
For the last couple of projects @larsvanbraam and myself used an adaptation from the hover mixin within our projects where we put the mixin in the global scope, and add the local selector to the mixin.
We've put this in asset/style/util/mixin/ and I think this would be a good idea to keep it by default there.
This will make sure the target is targeted correctly and we are easily able to update the check for when you want to support this custom hover functionality.
The mixin is as following:
Usage of this would be to put outside of the :local scope the following code:
We could remove the .no-touchevent, because the Modernizr set-up is empty by default. Or keep it as an option somehow.
Very curious on more ideas about this, and what you think.
The text was updated successfully, but these errors were encountered: