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

Prefix all CSS classes with hxr- #205

Merged
merged 3 commits into from
Jul 6, 2022
Merged

Conversation

humitos
Copy link
Member

@humitos humitos commented Jun 27, 2022

Avoid collisions with other CSS frameworks/themes/etc.

See executablebooks/sphinx-book-theme#577
Closes #180

@humitos humitos requested a review from agjohnson June 27, 2022 16:28
@humitos
Copy link
Member Author

humitos commented Jun 27, 2022

I opened this PR as an example showing what's the work required to prefix all the CSS classes with hxr-. The only work missing here is to define the prefix somewhere globally and re-use it from a variable or similar, instead of hardcoding it everywhere. In fact, we could make it a extension config if it's required.

@humitos humitos force-pushed the humitos/prefix-css-classes branch from 5a2b4cb to 4b7b72f Compare June 28, 2022 09:14
Copy link

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

This looks great!

One thing to keep in mind is that the Sphinx template file pattern will not work if we switch to our standard JS/stylesheet build system. I do see a few areas where this build system would add some features and help clean up the files used by users (plus it's using the same system we use elsewhere). But, we haven't found a pattern yet for post-processing the template variables back into the built assets. This is just a note for the future though, I'm sure there is an option we haven't explored yet.

@humitos
Copy link
Member Author

humitos commented Jul 6, 2022

Good note! I will probably hit that problem when working on the PR to include jQuery into the extension itself: #204. So, I will try to figure that out altogether. If it's not possible, I will probably remove the ability to change the prefix. At the moment, it's not exposed to users at least, which is good.

@humitos humitos merged commit 0250263 into main Jul 6, 2022
@humitos humitos deleted the humitos/prefix-css-classes branch July 6, 2022 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Hoverxref still not working on Jupyter Book?
2 participants