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

Media query dependent pygments style #7101

Closed
septatrix opened this issue Feb 6, 2020 · 14 comments · Fixed by #7142
Closed

Media query dependent pygments style #7101

septatrix opened this issue Feb 6, 2020 · 14 comments · Fixed by #7142
Labels
html theme type:enhancement enhance or introduce a new feature
Milestone

Comments

@septatrix
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently it is not easily possible to add an alternative pygments style based on a media query. This prevents simple implementations of a dark themes based on the prefers-color-scheme media query. The only way to do this currently is to hard code another css file with fitting overwrites which in not very user friendly and annoying if you want to change it.

Describe the solution you'd like
I would like an option inside the html section of the theme.conf file similar to the html_css_files. This option would then create additional pygments stylesheets, add them to the static files with a fitting postfix and link them as an html <link> element with a media attribute.

Describe alternatives you've considered
Manually linking the pygments stylesheets would require one to manually generate them which would not be fitting for a theme as it does not happen automatically in the build process.

Additional context
This would most likely require only small changes to create_pygments_style_file and init_highlighter

We stumbled upon this problem whilst implement a dark theme here python/python-docs-theme#44

@septatrix septatrix added the type:enhancement enhance or introduce a new feature label Feb 6, 2020
@tk0miya
Copy link
Member

tk0miya commented Feb 6, 2020

Thank you for proposal. TBH, I'm not familiar with CSS and media query. So I can't imagine what we should do. Could you make a detailed proposal please?

@septatrix
Copy link
Contributor Author

Currently we include <link rel="stylesheet" href="_static/pygments.css" type="text/css"> in our basic themes. If we now want to overwrite this stylesheet based on a media query we could just append a <link rel="stylesheet" href="_static/pygments-dark.css" type="text/css" media="(prefers-color-scheme)"> element after the above. If the media query is true this stylesheet is also respected and any color values referenced in there will overwrite the ones declared in the normal pygments.css file. This would need changes in the code generating the css files to also generate the pygments-dark.css file and a small change in the template to include the new stylesheet if enabled. An alternative would be to concat both stylesheets and wrap the later section in a corresponding media query but this is probably a bit more difficult and I don't know if browser optimize by not loading stylesheets with an unfulfilled media query...
I hope this helps :D

@septatrix
Copy link
Contributor Author

Shall I just go ahead and create a pull request of how I could think this can be archieved or is there something fundamental against implementing this?

@tk0miya
Copy link
Member

tk0miya commented Feb 8, 2020

Thank you for detailed explanation. I understand our goal. +1 for this idea. I'll think about how to implement it. Please let me know if you have idea!

@septatrix
Copy link
Contributor Author

I've pushed an Implementation in the above PR. As you can see it's quite easy to implement. To be mostly backwards compatible regarding config options and themes I opted to add a new html_ option and added the auxiliary styles to the normal css stylesheets. This makes it possible to also use this option in old themes. However it's not easy for a theme to provide defaults - as you may be able to see in the PR for python-docs-theme I linked above the only way for a theme to provide a default value for html_ values is to directly modify the default config options in the app object passed to the setup function..maybe there is a better way but I haven't been able to find one yet.

@tk0miya
Copy link
Member

tk0miya commented Feb 16, 2020

I wonder who'd like to use this feature. Users? Theme developers? Now your PR choose a configuration for this. It means this is mainly for users, right?
In addition, to disable default highlighting style is not needed?

@septatrix
Copy link
Contributor Author

I think this options will be used by both theme developers as well as end users.
Currently we would use this feature to add an automatic dark theme to the python docs (preview available under https://septatrix.github.io/cpython-dark-docs/index.html). However users may want to e.g. overwrite the dark syntax theme or add dark styles to a theme which does not support it by default. Therefore I think this option will be beneficial to both parties.

Regarding your other question - no I don't think disabling of default highlighting is needed if I understand your question correctly. As the auxiliary pygments styles are appended to the document after the default one they will overwrite it.

@septatrix
Copy link
Contributor Author

Additionally I would like to propose an analogous for a html_aux_stylesheet to add an additional stylesheet based on media queries. This option would supplement the normal stylesheet option and mainly for theme devs

@tk0miya
Copy link
Member

tk0miya commented Mar 8, 2020

Sorry for late. I think using a configuration is not good for developers. Because they can't touch it from their extensions and themes. For that purpose, it is better to add an option to theme.conf or a new API instead. Because it is difficult to touch a configuration value from code. (I know some extension accesses it via event handlers.)

I still don't understand that it is really useful for users to provide a such feature as a configuration. If my understanding is correct, it controls the style of code-block only. I suppose it is more important to support dark mode not only code-block, but also the whole of HTML theme. So I think no configurations are needed for this case, right?

@septatrix
Copy link
Contributor Author

They way it's currently implemented in my PR is using a config option accessible in theme.conf (for themes) and conf.py (for end users). If one of these options is set it will generate a second pygments style and add it to the html with the (prefers-color-scheme: dark) media query. Additionally I expose pygments_dark_style to the jinja context (as a boolean - but could also be changed to e.g. a string).

Themes could therefore provide a default dark syntax style (e.g. monokai) and users would be able to overwrite it if they prefer another one but don't want to create a new theme - as far as I know this more or less mirrors the behaviour of the pygments_style option.

Yes you are correct - this option only controls the generation of an additional pygments style and its inclusion in the html header. To support a complete html theme the user needs to add the code himself to their stylesheet. Unlike pygments styles though these are static and don't need to be generated therefore this won't be a problem and no configuration is needed.

Another possible configuration value to add would be a disable_dark_theme switch to give users a change to disable dark themes if needed though this would be a option theme developers would need to implement.

@tk0miya
Copy link
Member

tk0miya commented Mar 8, 2020

Sorry, I posted a comment above before reviewing your updated PR. I read your new PR and understand it includes new options for themes.

At present, I don't think users would not set the configuration because their expectation is controlling the mode of the whole of pages, not only code-blocks. So I think it is better to shrink the PR to provide the option for themes. It's not too late to make a configuration for users after making themes that use the new options. What do you think?

@septatrix
Copy link
Contributor Author

I think exposing a pygments_dark_style option will be beneficial to users but I agree that the option for other auxiliary themes most likely does not need to be exposed to users. If you want I can update the PR to remove that option. Shall I also include a disable_dark_theme or leave that out for now and leave that to theme developers?

@tk0miya
Copy link
Member

tk0miya commented Mar 8, 2020

I feel it is difficult to design the configurations for users at this moment. It is better to discuss them after the appearance of themes supporting dark mode. So I'd like to add only options for theme developers now.

@septatrix
Copy link
Contributor Author

Okay, I removed configuration options except the pygments_dark_style

@tk0miya tk0miya added this to the 3.0.0 milestone Mar 15, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
html theme type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants