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

Recognize :hover and put it at the end of selector #1942

Closed
mohebifar opened this issue Mar 31, 2014 · 13 comments
Closed

Recognize :hover and put it at the end of selector #1942

mohebifar opened this issue Mar 31, 2014 · 13 comments

Comments

@mohebifar
Copy link

It seems there's something unseen in LESS to me. I expect something like this in CSS:

.animate.fade-in, .animate-hover.fade-in:hover {
  animation-name: fade-in;
}

my LESS is :

.animate, .animate-hover:hover {
  &.fade-in {
    animation-name: fade-in;
  }
}

But what I get is :

.animate.fade-in, .animate-hover:hover.fade-in {
  animation-name: fade-in;
}

:hover's place is not right in output.

@lukeapage
Copy link
Member

This has come up before and maybe we should consider correcting the selector and moving the hover pseudo selector last. First I'd like to research if this should happen to every pseudo selector just a group of them. If its every pseudo I'd feel more inclined to do it.

@bcullman
Copy link

Doesn't this affect all pseudo selectors, not just :hover?

@matthew-dean
Copy link
Member

I don't think we should reshuffle :hover. There are definitely instances where the intended output is to affect (target) the class within the hover, and there's no way to know the author's intent to re-order / not re-order. I think the more appropriate solution would be to target / modify the parent selector ( See Issue #1075 ).

@seven-phases-max
Copy link
Member

there's no way to know the author's intent to re-order / not re-order

+1.

@lukeapage
Copy link
Member

@matthew-dean are you sure about that, can you give an example? This isn't shuffling pseudo selectors across elements, only on the same element so it appears after classes e.g. I don't think it changes the meaning (that's what I want to confirm), so a counter example would allow us to close.

@seven-phases-max
Copy link
Member

For the reference: .foo:hover.bar is valid CSS selector (though yes, for this particular example its meaning is equal to .foo.bar:hover).

@lukeapage
Copy link
Member

So is this just an aesthetic request?

@seven-phases-max
Copy link
Member

Probably it was not initially. Honestly I also did not know .foo:hover.bar is valid too until yesterday when I actually read the standard (which is blurish as always), run w3c validation tool and tested it in browsers to make finally sure :)


In fact I'm in great doubt about this feature, while I think it really could be something useful my concerns are the same as in more reasonable #1605 (where the result is actually an invalid CSS): by reshuffling we'll open that "I wrote A but actually I did mean B so can Less fix it for me?" pandora box with no evident point of where to stop and close it.

(Not counting those harsh sarcastic examples like "pozition: fixet;" - should Less fix that too? :) just a few additional remarks to consider:

(1). The same story applies to:

.animate  {
    &.fade-in {
        animation-name: fade-in;
    }
}

.animate-hover {
    &:hover:extend(.animate all) {}
}

Should we reshuffle this too?

(2). If such selector is written explicitly (e.g. just .foo:hover.bar {}), should it be "fixed" too?

(3). Implementation: For proper redordering we'll have to hardcode all the pseudo-elements in (to always put pseudo-elements after pseudo-classes), and there're not so few of them (e.g.) - we would not have to if pseudo-elements were used only with :: but they often are set with just :.

(4). I'm also concerned if such reshuffling will eventually make other features more complicated to implement, in particular extending dynamically generated classes and additional extend keywords
(Indeed, if we start to play with element order and consider .foo:hover.bar to be equal to .foo.bar:hover should not we also consider .foo.bar to be equal to .bar.foo etc. etc. etc.?)


And btw., all above w/o counting that there're several ways to get the desired styles without #1075 or so.

@matthew-dean
Copy link
Member

@lukeapage Yes, my concern was more similar to @seven-phases-max's. That is, in the example given, it seems reasonable, but a step in any direction, and the choices become less clear. I like when we exercise caution to make the parser, in general, output in a predictable and consistent fashion. If it reorders based on special text, it does fork output behaviors. I know we have to do that in some areas (like calc()), but I just think it's prudent to be conservative about behavior forking.

@matthew-dean
Copy link
Member

But, since you asked for it, I came up with a more obvious example:

.myClass:hover {
  &:before {
    foo: bar;
  }
}

The correct (and only valid) order is :hover:before. So if we special-cased :hover to stick it at the end (along with :visited, :focus, and all the rest of these?), we'd have to special-special-case exceptions for :before and :after and probably others. The complexity adds up quickly.

@mohebifar
Copy link
Author

I am not talking about appending pseudo-class or pseudo-element to the CSS selector. It is just about simple .class.

@matthew-dean
Copy link
Member

@mohebifar In your example, you stated ":hover's place is not right in output." Yet :hover is correct based on your usage of &, which I guess we assumed you meant that you wished for :hover to be appended (for & to operate differently in this case). Did we assume incorrectly? Were you just not sure how & works? What do you mean by "It is just about simple .class"?

@lukeapage
Copy link
Member

closing due to lack of response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants