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

Lint CSS code with stylelint #52

Merged
merged 2 commits into from
Jan 13, 2016
Merged

Lint CSS code with stylelint #52

merged 2 commits into from
Jan 13, 2016

Conversation

jeddy3
Copy link
Contributor

@jeddy3 jeddy3 commented Jan 13, 2016

Ref: #51

Adds stylelint to the npm run lint run-script.

I put the config inside package.json to match the existing babel and eslint configs.

There were a handful of changes to the CSS to bring it inline with stylelint-config-standard, mainly consistent leading zeros and hex color case.

@mxstbr
Copy link
Member

mxstbr commented Jan 13, 2016

LGTM

@jeddy3
Copy link
Contributor Author

jeddy3 commented Jan 13, 2016

Can we merge or does it need a second approval?

@mxstbr
Copy link
Member

mxstbr commented Jan 13, 2016

Needs a second LGTM. @MoOx?

Maybe we should set this to one LGTM, what do you think?

@marcustisater
Copy link
Member

LGTM

marcustisater pushed a commit that referenced this pull request Jan 13, 2016
Lint CSS code with stylelint
@marcustisater marcustisater merged commit f931518 into postcss:master Jan 13, 2016
marcustisater pushed a commit that referenced this pull request Jan 13, 2016
@marcustisater
Copy link
Member

Agreed, set it to one LGTM, I think two would be kept once the website is live.

@mxstbr
Copy link
Member

mxstbr commented Jan 13, 2016

Done with the latest commit, should be effective immediately. @jeddy3 we need another PR to try this on... ;)

@jeddy3
Copy link
Contributor Author

jeddy3 commented Jan 13, 2016

@jeddy3 we need another PR to try this on... ;)

On its way :)

@mxstbr mxstbr mentioned this pull request Jan 13, 2016
@marcustisater
Copy link
Member

😜

:global h5:hover .markdownIt-Anchor,
:global h6:hover .markdownIt-Anchor {
opacity: 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@jeddy3 is there a way to indent based on the "nesting" of the selector? (to keep this?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I believe the 'hierarchicalSelectors option' within the indentation rule would work here. But I figured it'd be better to use the actual nesting feature of postcss-cssnext and not have to extend and override the indentation rule within stylelint-config-standard. The CSS now looks like:

:global .markdownIt-Anchor {
  position: absolute;
  left: -1.5rem;
  opacity: 0.1;
  transition: opacity 0.2s;

  @nest :--headings:hover & {
    opacity: 1;
  }
}

It was changed in the follow-up PR that added postcss-cssnext to the boilerplate.

Sound good to you, or shall we take another look at it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good enough. Thanks for the clear explanation.

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

4 participants