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

Use :not(#\20) stackable root - more specificity #9

Closed

Conversation

iamstarkov
Copy link
Contributor

@iamstarkov iamstarkov commented Apr 25, 2017

Fix #8

Here I am with a PR 😄

Use :not(#\20) stackable root to get more specificity.

> Consider use :not(#\20), :not(.\20) and :not(\20)
> Rationale: \20 is a css escape for U+0020 Space, and neither classname, nor id, nor tagname can contain a space
> — https://twitter.com/subzey/status/829050478721896448

Fix MadLittleMods#8
return opts.stackableRoot.repeat(opts.repeat) + ' ' + selector;
});

if(opts.overrideIds) {
if(
// If an id is in there somewhere
(/#/).test(rule.selector) ||
(/#(?!\\)/).test(rule.selector) ||
Copy link
Contributor Author

@iamstarkov iamstarkov Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this thing was needed to be changed, because otherwise original regexp catches # in :not(#\20) and doesnt add !important

@iamstarkov
Copy link
Contributor Author

so, @MadLittleMods what do you think?

@iamstarkov
Copy link
Contributor Author

@MadLittleMods is something wrong?

@@ -16,7 +16,8 @@
},
"main": "index.js",
"scripts": {
"test": "mocha"
"test": "mocha",
"tdd": "mocha --watch"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use npm test -- --watch instead

@MadLittleMods
Copy link
Owner

Sorry for the delay @iamstarkov 😢

Reviewing now

@MadLittleMods MadLittleMods changed the title Feat/more specificity Use :not(#\20) stackable root - more specificity May 18, 2017
if(
selector === 'html' ||
selector === ':root' ||
selector === ':not(#\\20)' ||
Copy link
Owner

@MadLittleMods MadLittleMods May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should put this back to :root as it is root level thing . :not(#\\20) would be covered by the last case selector === opts.stackableRoot

[idea]: https://twitter.com/subzey/status/829050478721896448
[@subzey]: https://github.com/subzey
[@iamstarkov]: https://github.com/iamstarkov
[#9]: https://github.com/MadLittleMods/postcss-increase-specificity/pull/9
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer inline linking vs references. Seems fine to keep it for now as it will be changed in the release.

@MadLittleMods
Copy link
Owner

@iamstarkov I'll take care of the remaining points (no need for a further fix commit).

@MadLittleMods
Copy link
Owner

Merged via #10

Thanks @iamstarkov 💓 😺

Appreciate the ping again to take a look. This was sitting in my inbox for too long.

@MadLittleMods
Copy link
Owner

Published in postcss-increase-specificity@0.4.0, https://www.npmjs.com/package/postcss-increase-specificity

@iamstarkov iamstarkov deleted the feat/more-specificity branch May 19, 2017 10:28
@iamstarkov
Copy link
Contributor Author

@MadLittleMods thank you

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.

2 participants