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

Convert prism-tomorrow.css to stylus and include it in our main CSS. #2462

Merged
merged 1 commit into from
Sep 20, 2019
Merged

Convert prism-tomorrow.css to stylus and include it in our main CSS. #2462

merged 1 commit into from
Sep 20, 2019

Conversation

XhmikosR
Copy link
Contributor

Requires #2461!

The idea is to combine the Prism CSS with our main CSS and thus run autoprefixer and minifier like our main CSS file.

I know it's a 3rd-party file and we probably shouldn't touch it, but this way we end up with less CSS, which has the same transformations applied as our main CSS file.

Let me know your thoughts. I tried just @importing the Prism CSS file in our styl file, but then it wasn't being minified. I'll give that another go later, because this solution would be the least intrusive one.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Sep 8, 2019

Not sure how to proceed with this. If we go with #2530, the stylus conversion is moot, but #2530 is built on top of this PR. Also, I'm not sure if or when #2530 will land, so perhaps we should move with this in the meantime?

@XhmikosR
Copy link
Contributor Author

Since I have based my PRs on this, I think it makes sense to move on with this for the time being.

When the Sass PR is merged, the prism CSS file will be restored.

@XhmikosR XhmikosR marked this pull request as ready for review September 18, 2019 06:33
@Trott
Copy link
Member

Trott commented Sep 18, 2019

@nodejs/website


code[class*='language-'],
pre[class*='language-']
color #ccc
Copy link
Contributor

Choose a reason for hiding this comment

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

$light-gray2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a 3rd-party file, I'd rather keep it close to the original.

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.

4 participants