Suppress 'rtd-footer-container' if not being built on ReadTheDocs

[DocOtak]

This PR attempts to fix #1794 and possibly #1238 by not including the rtd-footer-container div if the site is not being built on ReadTheDocs.

This uses the same {% if READTHEDOCS %} as the ethical ads html.

[DocOtak]

Two alternatives to this I think are either to give the rtd-footer-container a min-height of 1rem. Or set the bottom margin to 0. The negative top and bottom combined with the zero height of this div when there is no content has the effect of pulling the div 1rem bellow its parent's bottom.

edit: I think doing the min-height option might be more correct for what I think is intended

[Carreau]

This looks good to me, I guess I don't really like magic variable for which are not defined anywhere, and would prefer isdefined(...) or similar. But I think this will do.

I've approved the workflow and will wait for tests to pass.

[Carreau]

I think I want @gabalafou review on this, and if

function initRTDObserver()

needs update as

document.getElementById("rtd-footer-container").append(...)

Risk raising null has not attribute append, but I'm no js expert.

I thinks some tests needs updating as they assume the div exists (when comparing to expected files)

[Carreau]

test files needs updates.

[DocOtak]

@Carreau I also don't like magic variables and had trouble figuring out where {% if READTHEDOCS %} even comes from. Would you prefer the min-height solution? It would leave the DOM untouched and still fix the scrolling issue.

[DocOtak]

I managed to get the test running locally, thank you so much for providing a devcontainer definition!

I'm not sure how this PR might affect #1398 since that does away with the need for this container div that is causing problems.

[Carreau]

Would you prefer the min-height solution? It would leave the DOM untouched and still fix the scrolling issue.

No, this solution is great; I just don't generally like those magical variables.

Don't worry about #1398 , it's been dormant for some time; and I think it's easy enough to adapt if the need arises.

[Carreau]

+1, just want some JS experts input, but if there is not answers in a week or so I think it's good to merge.

[Carreau]

+1, just want some JS experts input, but if there is not answers in a week or so I think it's good to merge.

[github-actions]

Coverage report

This PR does not seem to contain any modification to coverable code.

[gabalafou]

I looked into this today, and discovered some important facts:

• In 2023, Read the Docs Addons introduced, changing the way the version switcher is injected
• Last month, on October 7, 2024 - Addons enabled for all projects
• As a result, none of the code that @12rambau added still works: ENH: place the RDT flyout in the sidebar #1005

So I think the right way forward is to remove all of that code. I opened a pull request to do so:

• Stop moving the Read the Docs version switcher into the sidebar #2034

If folks agree that that pull request is the right way forward, then we can close this pull request without merging.

Thank you for helping to make PyData Sphinx Theme better!

[DocOtak]

@gabalafou Even better than what I've done, definitely in favor of merging #2034 in favor of this.

[Carreau]

Closing in favor of #2034