Skip to content

Prevent pseudo selector args from prefixing #5

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

Merged
merged 1 commit into from
Jul 3, 2016

Conversation

toddself
Copy link
Contributor

@toddself toddself commented Jul 3, 2016

This change prevents the argument to a pseudo selector from being prefixed
(since it shouldn't be!).

Also added provisions to run the tests via npm test

I'm not sure if this is the best way to solve this, but it does fix #4.

We might not care that the first parent selector is a selector and only need to determine if the parent's parent is a pseudo selector.

I still feel like this might be a bug in the parser, however, it's late and I don't have time to delve into that :)

@ahdinosaur
Copy link
Member

hey @toddself

this pull request is awesome, thanks for working on this. 😸

code changes look good, but to honest i'm not sure i completely understand what's happening here. css is confusing, it's a mystery how my previous self even wrote the current code here. so just wondering, are there any other test cases you can think of that might fall into this same bug?

anyways, great work!

@toddself
Copy link
Contributor Author

toddself commented Jul 3, 2016

not a problem!

Any of the CSS psuedo selectors which take an argument:

  • :dir
  • :lang
  • :nth-* (last-child, child, of-type, last-of-type)

The difficulty here, I realize, is that :not should get it's arguments prefixed:

.myel :not(p) {}

Should likely transform to:

#hello-world .myel :not(#hello-world p) {}

Let me update the PR to handle this case and add cases for other elements as well just to make sure our t's are crossed and i's dotted

This change prevents the argument to a pseudo selector from being prefixed
(since it shouldn't be!).

Also added provisions to run the tests via `npm test`
@toddself
Copy link
Contributor Author

toddself commented Jul 3, 2016

OK this has been updated to test all the pseudo selectors which take arguments and handle :not correctly with a prefix.

The logic feels a little gnarly, but I'm not sure there's a better way since we're looking at the argument to the selector when we need to make the determination?

Anyway, the tests pass :)

@yoshuawuyts
Copy link
Contributor

Looks quite reasonable to me; everything seems tested now - merging! ✨

@yoshuawuyts yoshuawuyts merged commit 83e8300 into stackcss:master Jul 3, 2016
@yoshuawuyts
Copy link
Contributor

Published as v1.0.4

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.

prefixes shouldn't be applied to psuedo-class arguments
3 participants