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

Download page: Added options #2162

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

RunDevelopment
Copy link
Member

This adds a new 'Options' section to the download page where plugins and Prism itself can be configured.

image


The basic idea here is that there are options which are a collection of option items. An option can require any number of components and is only enabled if all required components are enabled. Only options that are enabled will be visible.

Every option item translates to one label + one input element. An item is responsible for validating inputs. I added support for 3 types of inputs: boolean (-> input[type=checkbox]), number (-> input[type=number]), and string (-> input[type=text]). But right now, I don't use the number type.

The option will then be used to apply changes to the JS and CSS source code according to the values of its items.

The item values of all enabled options will be stored in the URL if the value differs from the default value.


I added 3 options:

  1. A small general option to disable automatic highlighting.
  2. The Custom Class prefix option. ([Brainstorming] Decouple token names and themes #1055)
  3. Options for Filter HighlightAll to set filterKnown and to add CSS selectors. (not enabled in the image)

To apply the prefix to the theme, I used Prism's CSS language to tokenize the CSS code and get all selectors and replaced all token names using a regular expression.

@LeaVerou
Copy link
Member

This is fantastic work @RunDevelopment! I haven't looked at the code very closely, but the functionality is sorely needed!

@RunDevelopment
Copy link
Member Author

Thanks.
Looking at it again, the only thing I'm unhappy about is that download.js now contains plugin logic. I really should have made the getOptions function into its own file (this will also help the already quite large download.js).

And I need to revisit the Custom Class implementation. I used Prism to parse CSS because it was convenient but come on. I should do this the right way.

@LeaVerou
Copy link
Member

And I need to revisit the Custom Class implementation. I used Prism to parse CSS because it was convenient but come on. I should do this the right way.

I did that a few days ago 😊 (to parse JS though, not CSS)

@RunDevelopment
Copy link
Member Author

Ok. No CSS parser. I wasn't able to find a good CSS parser that just works on the web and preserves spaces and comments. Prism-parsing it is then.

I also found another major pain point for the Custom class prefix option: To which classes do we add the prefix?
Right now, I add the prefix to all classes that are in the same component selector as a .token class. This works for all themes but breaks some plugins. It breaks the CSS of plugins in two ways:

  1. Some component selectors of tokens don't have a .token class. I think that this is a problem of the plugin and should be fixed for each respective plugin. I know that Tree view has this problem. I haven't check for any other plugins yet.
  2. Some plugins might all classes to tokens after the warp hook meaning that Custom Class can't add the prefix. The CSS will still get the prefix, so the style doesn't work (e.g. Match braces). How do we fix this? (Match braces is incompatible with Custom class right now anyway.)

@LeaVerou
Copy link
Member

Ok. No CSS parser. I wasn't able to find a good CSS parser that just works on the web and preserves spaces and comments. Prism-parsing it is then.

You can also just do regex replace on the fetched CSS, theme CSS is pretty simple, so that should work quite well.

I also found another major pain point for the Custom class prefix option: To which classes do we add the prefix?

Would not prefixing the token class itself alleviate any of these? I suspect it's not a very commonly used name...

Also, perhaps we need to bite the bullet and bake this functionality into the core instead of making custom classes a plugin. It seems like fairly important functionality…

(Aaand this is where I wish we had usage stats for plugins...)

@RunDevelopment
Copy link
Member Author

Would not prefixing the token class itself alleviate any of these?

Not really. Most plugins search for specific tokens by their type. Given that Prism supports arbitrary prefixes and even remapping (via Custom Class), you could argue that the plugin (e.g. Match Braces) is at fault.

Right now, the only thing nobody modifies during the conversion from token stream to markup are attributes, so I could use marker attributes instead of marker classes to implement Match Braces. The plugin will then be implemented partly in warp and partly in complete. In wrap I'll add the marker attributes before Custom Class changes any classes and in complete I'll query by these attributes. Should work, I guess.

To make this more stable we could make a pre-wrap hook that isn't allowed to change data in the environment but it can append new data (of course, you can also read). I.e. you can't change any classes or attributes in pre-wrap but you can add new ones.

(Aaand this is where I wish we had usage stats for plugins...)

We don't have those? I always assumed that we did. I'm not very familiar with Google Analytics but if they also collect information on the hash of the URL of the download page, then we have these stats.

@web-apply

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants