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

Include Mathjax.js on per-page basis #6241

Closed
jabl opened this issue Apr 1, 2019 · 14 comments
Closed

Include Mathjax.js on per-page basis #6241

jabl opened this issue Apr 1, 2019 · 14 comments
Labels
extensions type:enhancement enhance or introduce a new feature
Milestone

Comments

@jabl
Copy link

jabl commented Apr 1, 2019

Is your feature request related to a problem? Please describe.

Currently in a documentation project we have many pages, of which only a very small subset needs mathjax. However, enabling the sphinx.ext.mathjax extension causes mathjax.js to be included in every page. This is a bit unfortunate since mathjax is huge.

Describe the solution you'd like

Mathjax.js is included in only the pages which need it, and not every page.

Describe alternatives you've considered

Current system works, but makes all pages that don't need mathjax bigger and slower than they need to.

Additional context

I mentioned this as a comment in #5497 , where it was suggested that I open a new issue.

@jabl jabl added the type:enhancement enhance or introduce a new feature label Apr 1, 2019
@tk0miya tk0miya added this to the 2.1.0 milestone Apr 1, 2019
@tk0miya tk0miya modified the milestones: 2.1.0, 2.2.0, 3.0.0 May 28, 2019
@tk0miya tk0miya modified the milestones: 3.0.0, 3.1.0 Mar 14, 2020
@tk0miya tk0miya modified the milestones: 3.1.0, 3.2.0 May 30, 2020
@tk0miya tk0miya modified the milestones: 3.2.0, 3.3.0 Aug 1, 2020
@pradyunsg
Copy link
Contributor

executablebooks/sphinx-panels#28 (comment)

An idea for a Sphinx plugin that does what's been requested here.

@akhmerov
Copy link

akhmerov commented Oct 7, 2020

Thinking about possible plugin implementations, would hooking into html-page-context be a good approach here? This is called on a per-page basis, and has the advantage that the content isn't mutated afterwards.

In order to not change the order of libraries and stylesheets included, I think conditionally removing js would work better, and the plugin configuration could specify a doctree node selector that is required to keep a js/css file.

@pradyunsg
Copy link
Contributor

Yup -- that's exactly what I had in mind -- filtering the JS files "out" based on some conditional for them. Something like "only remove if there's no .math elements".

The exact design is 100% in the air though -- I imagine callable functions maybe nice in the spirit of extensibility but that's complexity and so on. :)

@akhmerov
Copy link

akhmerov commented Oct 7, 2020

Thinking about sphinx design, I'd say using any condition accepted by doctree.traverse should do. That would allow both the simple option of specifying node types, and arbitrary more complex ones.

@choldgraf
Copy link
Contributor

choldgraf commented Oct 7, 2020

I think the pattern you suggested to remove files would work. I just tried adding an html-page-context listener to conditionally remove another JS library (thebelab) when a page didn't have the node that would trigger it, and it seemed to work nicely 👍

Here's the code I used in case it's helpful:

def remove_thebe_if_not_needed(app, pagename, templatename, context, doctree):
    if not doctree:
        return

    if not doctree.traverse(ThebeButtonNode):
        # Remove thebe JS files
        new_script_files = []
        for ii in context["script_files"]:
            if ii not in ["_static/sphinx-thebe.js", "https://unpkg.com/thebelab@latest/lib/index.js"]:
                new_script_files.append(ii)
        context["script_files"] = new_script_files

        # Remove thebe CSS files
        new_css_files = []
        for ii in context["css_files"]:
            if ii not in ['_static/thebelab.css', '_static/sphinx-thebe.css']:
                new_css_files.append(ii)
        context["css_files"] = new_css_files

@akhmerov
Copy link

akhmerov commented Oct 7, 2020

Sweet! Now the real question is if it's even worth an extension of its own—the amount of code is minimal.

It would potentially make sense to make it a part of sphinx itself by allowing js and css files also specify a doctree selector for themselves to be included.

@akhmerov
Copy link

akhmerov commented Oct 7, 2020

Not sure what would be a good interface. The obvious option is to allow

html_js_files = [
    'js/custom.js',  # included unconditionally
    ('js/large_optional_library.js', library_selector),  # only if selector isn't empty
]

This is backwards compatible, but ugly.

@choldgraf
Copy link
Contributor

choldgraf commented Oct 7, 2020

hmmmm, I will think about it a bit more...another option is something like:

  • an extension could provide its own function to add files like add_conditional_js_file or something
  • that would have all the same arguments as add_js_file except for another one called condition which would be a callable that takes all of the arguments of html-page-context
  • The extension adds an event callback for html-page-context. In that callback, the user-provided callable is called.
  • If the function returns not None then it does nothing.
  • If the function returns None, then it removes the JS/CSS file from the page context

Then extension authors could do whatever they want with callable. E.g. some might want the JS loaded only on pages that meet a user-provided variable like load_js_on_pages = ["index", "config"]. Others might want to parse the doctree. Others might want to check for a context variable like load_library=True.

@pradyunsg
Copy link
Contributor

  • If the function returns not None then it does nothing.

  • If the function returns None, then it removes the JS/CSS file from the page context

I'd use boolean values instead, but otherwise I like it.

If we're willing to be a bit... uhm... YOLO about this, we could even have the condition argument be added as a kw-only argument to add_js_file. I don't imagine anyone is setting that attribute on a JS file.

@akhmerov
Copy link

akhmerov commented Oct 7, 2020

Does add_js_file cover the user-provided html_js_files?

Also: should the condition take all arguments passed to html-page-context or just a doctree traverse selector (that one is also callable and may do arbitrary stuff on the doctree).

@Pike
Copy link

Pike commented Nov 5, 2020

What if we substituted

app.connect('env-updated', install_mathjax)

for

app.connect('html-page-context', install_mathjax)

That would provide the page name, and then the check could dive into whether that page has math tags?

@choldgraf
Copy link
Contributor

I am prototyping this over here, if any others have thoughts or contributions: https://github.com/executablebooks/sphinx-conditional-asset/blob/main/sphinx_conditional_asset/__init__.py#L11

My plan is to release it as an extension once we have a pattern that makes sense, and then if Sphinx core thinks it'd be good to have, upstream it.

@tk0miya tk0miya modified the milestones: 3.4.0, 3.5.0 Dec 19, 2020
tk0miya added a commit to tk0miya/sphinx that referenced this issue Dec 31, 2020
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-context-page`
event.
@tk0miya
Copy link
Member

tk0miya commented Dec 31, 2020

I posted #8631 to support JS/CSS files for the specific page. Please take a look and let me know your opinion for the change.

@tk0miya tk0miya modified the milestones: 3.5.0, 4.0.0 Dec 31, 2020
tk0miya added a commit to tk0miya/sphinx that referenced this issue Dec 31, 2020
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-context-page`
event.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Dec 31, 2020
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-page-context`
event.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Jan 2, 2021
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-page-context`
event.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Jan 2, 2021
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-page-context`
event.
@tk0miya tk0miya modified the milestones: 4.0.0, 3.5.0 Jan 4, 2021
tk0miya added a commit to tk0miya/sphinx that referenced this issue Jan 4, 2021
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-page-context`
event.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Jan 4, 2021
…c page

Allow to add JS/CSS files to the specific page when an extension calls
`app.add_js_file()` or `app.add_css_file()` on `html-page-context`
event.
@tk0miya tk0miya closed this as completed in af4e615 Jan 7, 2021
tk0miya added a commit that referenced this issue Jan 7, 2021
Close #6241: html: Allow to add JS/CSS files to the specific page
@jabl
Copy link
Author

jabl commented Jan 7, 2021

(Original reporter here). Thank you very much!

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

No branches or pull requests

6 participants