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

Add plugin overrides for One Dark and One Light #134

Merged
merged 23 commits into from
Sep 14, 2021

Conversation

hoonweiting
Copy link
Contributor

Updating One Dark and One Light with plugin overrides!

Currently a work in progress! I noticed an issue filed for a new release, hence the draft as a head's up. Hopefully I'll be done in the next few hours.

@RunDevelopment
Copy link
Member

Thank you for making this @hoonweiting!

May I ask which plugins you intend to improve?

Hopefully I'll be done in the next few hours.

Don't feel pressured and take your time. The release can wait for another day (or week).

@hoonweiting
Copy link
Contributor Author

I'm looking at Show Invisibles, Toolbar, Line Highlight, Line Numbers, Diff Highlight, Command Line, Match Braces, and Previewers. I might drop some of them though! I'll see how it goes.

Thank you for your support :)

@RunDevelopment
Copy link
Member

Wow. That's pretty much all of them. Thank you!

If you want to prioritize certain plugins, let me tell you that Line Numbers, Line Highlight, and Toolbar are the most popular ones in that order.

@hoonweiting
Copy link
Contributor Author

Ah that's great to know, thank you! :)

@hoonweiting hoonweiting mentioned this pull request Sep 12, 2021
@hoonweiting
Copy link
Contributor Author

I'm about done, but to be certain I want to double check again tomorrow before submitting the PR, it's almost 4am here oops 💤

@hoonweiting
Copy link
Contributor Author

I think I'm done with the plugin overrides! :)

@hoonweiting hoonweiting marked this pull request as ready for review September 13, 2021 17:53
@hoonweiting
Copy link
Contributor Author

I have increased the selector specificity of most of them by at least (0, 0, 1). (The only one that didn't get increased does not override, and instead just sets a new property.) Maybe repeating a class would have been easier for maintenance though, hmm, not sure if I should change it.

@RunDevelopment
Copy link
Member

RunDevelopment commented Sep 14, 2021

The only one that didn't get increased does not override, and instead just sets a new property.

Plugins may (need to) add additional properties in the future. So increasing the specificity for all plugin overrides is probably a good idea.

Maybe repeating a class would have been easier for maintenance though, hmm, not sure if I should change it.

I think repeating a class would be better.

I know that it looks silly but that is actually a good thing. It makes it clear that there is something special about that selector. With your current approach, it's not possible to see which parts of the selector are necessary for selecting the element and which are there to increase the specificity.

It also has the advantage that there are no new requirements.

(There's also the very minor advantage that repeating classes is easy for gzip to compress.)

@hoonweiting
Copy link
Contributor Author

Right, gotcha! I'll work on it in a bit.

@RunDevelopment RunDevelopment merged commit e6cf989 into PrismJS:master Sep 14, 2021
@RunDevelopment
Copy link
Member

Thank you very much for contributing @hoonweiting!

I'll make a new release tomorrow or the day after.

@hoonweiting hoonweiting deleted the one-dark-one-light branch September 14, 2021 19:07
@hoonweiting
Copy link
Contributor Author

Alright! I'll look into the plugin overrides for the other existing themes as well, shouldn't take too long since I've already ironed out a sample of sorts.

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