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

Remove dependence on JQuery or explicitly include it in the theme #764

Closed
choldgraf opened this issue Jun 27, 2022 · 8 comments · Fixed by #1029
Closed

Remove dependence on JQuery or explicitly include it in the theme #764

choldgraf opened this issue Jun 27, 2022 · 8 comments · Fixed by #1029
Labels
kind: enhancement New feature or request
Milestone

Comments

@choldgraf
Copy link
Collaborator

choldgraf commented Jun 27, 2022

Context

Sphinx 6.0 is going to remove the jquery dependency by default. We use jquery a little bit in this theme, though likely not in a way that strictly requires jquery.

Here's a GitHub CS search for the $( symbol

Proposal

We should try removing any explicit dependence on jquery and replace it with document.querySelector etc if possible. If we hit some hard blocker that requires jquery, we should discuss whether to work around it or re-introduce the jquery dependency.

If we need to keep JQuery, we'll need to bundle it ourselves with this theme, and remove it from externals here:

externals: {
// Define jQuery as external, this way Sphinx related javascript
// and custom javascript like popper.js can hook into jQuery.
jquery: "jQuery",
},

@choldgraf choldgraf added the kind: enhancement New feature or request label Jun 27, 2022
@choldgraf choldgraf changed the title Remove dependence on JQuery Remove dependence on JQuery or explicitly include it in the theme Jul 9, 2022
@12rambau
Copy link
Collaborator

I started to look into this one and it seems bootsrap 4 is exclusively relying on jquery for js behaviours. What do you think if we update the theme requirements to use bootstrap 5 ?

@choldgraf
Copy link
Collaborator Author

I'm all for upgrading to bootstrap 5. We should probably track that in it's dedicated issue though because it might involve some discussion or decisions.

For jquery I believe there is some work on an extension that we can use to handle the jquery dependency as a short term fix (cc @humitos)

@humitos
Copy link

humitos commented Oct 6, 2022

For jquery I believe there is some work on an extension that we can use to handle the jquery dependency as a short term fix (cc @humitos)

Thanks for the ping on this issue. We did some progress around the Sphinx jQuery extension we wanted to create and we already have a plan. We have it targeted for Q4, so I'd say that we will start working on this in the next few sprints.

Note that Sphinx 6.x release (including jQuery removal) may change its dates. In that case, we may need to pin Sphinx<6 to avoid breaking the theme. We are talking about that at sphinx-doc/sphinx#10896

@AA-Turner
Copy link
Contributor

We did some progress around the Sphinx jQuery extension we wanted to create and we already have a plan.

Is this solely an extension that has app.add_js_file(<jquery>)? If so, I'm happy to publish this under the sphinx org later this week. If anything more complicated I'd need a little more info!

A

@benjaoming
Copy link

@AA-Turner I work with @humitos and we're playing a bit of ping-pong on coming up with an ideal solution for handling jQuery assets. We're quite keen on fixing the issue before it turns into actual problems, but we also believe that it should be a broader community effort.

We've opened up our proposal here: readthedocs/readthedocs.org#9665

I'm happy to publish this under the sphinx org later this week.

Would you mind elaborating what you mean here? We'd be very happy to see the proposal implemented by Sphinx maintainers. If you agree with the proposal or you have any additions or simplifications, we'd be very happy to hear from you. In case you understand it want to move forward with it, that's also 👌 👍

@AA-Turner
Copy link
Contributor

Hi Ben,

I didn't hear back from you so I was working on this last night with the 6.0 beta -- I've put what I had up at https://github.com/sphinx-contrib/jquery

I'm inclined to make a release to PyPI to reserve the name, too.

It would be great to work on this with the RTD team if you're interested, I can add you as a committer to the repo/etc!

A

@benjaoming
Copy link

Hi A

I think it would be great if you can add me and @humitos to the new repo.

Best,
Ben

@benjaoming
Copy link

Everyone else reading: There are some more changes and comments on the proposal - readthedocs/readthedocs.org#9665

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants