-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Redundant readthedocs-notification
and readthedocs-flyout
are not placed correctly on some projects
#11136
Comments
Hi, the project differt has our new addons feature enabled. You can disable it from https://beta.readthedocs.org/dashboard/differt/addons/edit/. But since our new implementation is enabled, we shouldn't show the old warning implementation. |
@humitos is this a known bug? When addons are enabled, we are still injecting/showing the external version warning from our sphinx extension. I guess our extension needs to know if addons are enabled and don't inject the warning anymore. |
Ouch, you are right! I feel a bit stupid that the fix was that simple... I activated all the beta add-ons a bit blindly a few weeks ago, not really knowing what it meant. I guess this would be great to add a short explanation for each of the box you can tick. Should I close this issue? Or keep it open, as it clearly shows some shortcomings of the beta add-ons? Thanks! |
Let's keep it open till we get an answer from humitos.
I think this is a good idea. |
I know we used to have a similar bug, but we should have fixed it already. If it's still there, I'm sure we will need to make an adjustment to the query selector to catch the warning and remove it (see https://github.com/readthedocs/common/blob/8ad2d974abba545f27a9193907ddc0dda2377ef2/dockerfiles/force-readthedocs-addons.js#L40-L41) @jeertmans can you try enabling the addons again on that project so I can debug this further and work on a fix? |
Yes, we are working on expanding our documentation around Read the Docs Addons and also adding more context on that admin page. This work is already in our roadmap 👍🏼 |
@humitos just re-enabled it, this is now ugly again :'-) |
@jeertmans thanks! Can you please keep it enabled for now? I will take a look at this tomorrow. For tomorrow: this is the page where the "old warning" appears and it should not: https://differt--50.org.readthedocs.build/50/ |
I found the issue. Our CF worker is not removing the "Warning" admonition because the selector is not catching it. We are using https://github.com/readthedocs/common/blob/8ad2d974abba545f27a9193907ddc0dda2377ef2/dockerfiles/force-readthedocs-addons.js#L40-L41 It seems the Sphinx theme you are using doesn't declare We made it the selector very specific because we had an issue where we were removing warnings that weren't created by Read the Docs:
We should find a way to adapt this selector while making sure we are removing what we want to remove. |
Note that the issue also appeared with the Furo theme, both very popular themes I think. Can I disable the addons, or do you still need them? |
I suppose you mean Furo here, and it was just a typo. Do you have an example of a project using Furo theme with addons enabled experimenting this issue as well? |
Yes indeed this is Furo! 😅 |
In readthedocs/readthedocs-ops#1429 we made the selector to remove the "external version warning" a lot more specific. It fixed the original issue, but it resulted in not matching in other themes. This PR adds extra specific CSS selector for Furo and Book themes. Closes readthedocs/readthedocs.org#11136
In readthedocs/readthedocs-ops#1429 we made the selector to remove the "external version warning" a lot more specific. It fixed the original issue, but it resulted in not matching in other themes. This PR adds extra specific CSS selector for Furo and Book themes. Closes readthedocs/readthedocs.org#11136
@jeertmans I deployed a small fix for this and did a small successful tests on those projects. Can you confirm the old warning notification is removed now while keeping the addons enabled? BTW, the URL for the Furo project I'm testing over is https://manim-slides--374.org.readthedocs.build/374/ |
I can confirm, thanks! The version flyout is at a different place than usual (bottom right corner instead of left sidebar), but I guess this is on purpose? |
Yes, that's fine. We are working on better integrations for different themes. If you are interested in following that work, you can subscribe to readthedocs/sphinx_rtd_theme#1526 |
Done! Thanks for your help, I think I can safely close this issue now :) |
In readthedocs/readthedocs-ops#1429 we made the selector to remove the "external version warning" a lot more specific. It fixed the original issue, but it resulted in not matching in other themes. This PR adds extra specific CSS selector for Furo and Book themes. Closes readthedocs/readthedocs.org#11136
Details
Hello!
For both of my project, Manim Slides and DiffeRT, I observe that the versions flyout is not placed in the left sidebar, but in the bottom right corner of my page (see second image).
Also, on PR or non-stable builds, I observe a (second) notification banner, that appears on every page, even if I already have some warning (see screenshots) placed by RTD.
After hours of debugging, I actually observed that it only affected my DiffeRT and Manim Slides project.
Let's take the example of
jupyter-server-proxy
:FORK
💚 I forked their repo and rendered my own version on RTD, and it renders as expected;DIFFERT
💔 I CTRL+C/CTRL+V their whole repository into mine, i.e., DiffeRT, pushed that on a separate branch and created a PR, and the build does not render as expected (see below).Note that the configuration on RTD are exactly the same for
FORK
andDIFFERT
repository, except that I use a custom domain forDIFFERT
.Expected Result -
FORK
Here is what happens if a build from a PR, on a fork of a project I know has working docs.
As you can see, I obtain the same results as from the original repo, see my PR build: https://jupyter-server-proxy-jeertmans--1.org.readthedocs.build/1/.
Actual Result -
DIFFERT
But, as soon as I build from my other repository
DIFFERT
, I can the followingNote
I did clear the browser cache, try in incognito window, none of those removed
the white banner nor placed the version correction.
Further details
Looking at the page sources, they also are different:
The following source files are loaded on
FORK
, but not onDIFFERT
:readthedocs-doc-embed.css
andreadthedocs-doc-embed.js
;badge_only.css
.I guess this may explain the difference, but again I don't know what triggers that and, more importantly, how to fix this issue 😕
I hope I gave enough details about my issue. If not, please do not hesitate to tell me!
Thanks in advance for your help!
The text was updated successfully, but these errors were encountered: