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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(code): add a way to configure Prism theme #38

Closed
wants to merge 2 commits into from

Conversation

@Kocal
Copy link
Contributor

Kocal commented Jan 8, 2020

Summary

I didn't like Prism theme okaidia and I wanted to use another one, but it was not possible because the theme was hardcoded.
I've added a Stylus variable $prismTheme (default okaida) and changed the way to include a prism theme (directly import it in code.styl, like default Vuepress theme does).

default theme:
S茅lection_999(508)

(Don't take care of highlighted lines, it's part of #37 馃槄)

override theme:
S茅lection_999(509)

I've also updated the documentation:
S茅lection_999(510)

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome 78.0.3904.108
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated (I don't think there are tests for that)

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@@ -133,3 +133,5 @@ div[class~="language-bash"]:before

div[class~="language-php"]:before
content "php"

@import '~prismjs/themes/prism-' + $prismTheme + '.css'

This comment has been minimized.

Copy link
@newsbielt703

newsbielt703 Jan 8, 2020

Member

Prism's default theme @import '~prismjs/themes/prism.css won't be able to configure.

This comment has been minimized.

Copy link
@Kocal

Kocal Jan 8, 2020

Author Contributor

Ah, nice catch

@newsbielt703

This comment has been minimized.

Copy link
Member

newsbielt703 commented Jan 8, 2020

For now, you can use index.stly to override it, such as:

//index.stly

@import '~prismjs/themes/prism-twilight.css'

Since our code block background is always black, some theme might not be suitable.

AFAIK, VP's default theme doesn't support configuring Prism theme. I can't confirm whether it should be supported, but it's worthwhile to open an issue in VuePress, and we can follow the decision.

@Kocal

This comment has been minimized.

Copy link
Contributor Author

Kocal commented Jan 8, 2020

Yes, VP's default theme does not support configuring Prism theme, they directly use @import '~prismjs/themes/prism-tomorrow.css' in styles/code.styl.

Using your solution won't work properly because Prism is still imported with <style src="..."> in this theme. It seems that Prism need to be imported with @import in Stylus in the base theme in order to be overridable, see vuejs/vuepress#1707.

I will open a new PR to remove <style> and import Prism in file code.styl, like it's done in VP's default theme.
Like this, end users will be able to override it in their index.styl.

@Kocal Kocal closed this Jan 8, 2020
@Kocal Kocal deleted the Kocal:fix/prism-configuration branch Jan 8, 2020
@Kocal Kocal mentioned this pull request Jan 8, 2020
3 of 18 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.