EthicalAds: use theme-logic to decide position

[humitos]

The logic/idea is borrowed from the old implementation at
https://github.com/readthedocs/readthedocs.org/blob/main/readthedocs/core/static-src/core/js/doc-embed/sponsorship.js#L51

Related https://github.com/readthedocs/addons/pull/295/files#r1600336686

[humitos]

This is how it works now:

It seems there may be something wrong with the classes because it doesn't render properly when it goes to the navbar. Any clue here?

[humitos]

This is how it works in Alabaster-like themes.

[humitos]

Hrm... I was missing including the CSS file 👍🏼 -- it should work after I figure it out how to include it without using web components 😅

[humitos]

With the correct CSS it looks 👍🏼 , but I'm not sure yet why it's not being loaded by default:

[humitos]

It seems it figured it out. I only need to understand why the css that I'm injecting via

addons/src/ethicalads.js

Line 4 in 2e64849

import styleSheet from "./ethicalads.css";

does not affect the ad. I think @agjohnson may have an idea here.

[humitos]

However, activating the CSS manually looks great!

[davidfischer]

Some of this is historical. When the ad appears in the stickybox in the lower right corner, it uses the CSS directly from the ad client. However, when it appears in the sidebar, it's usually using some custom RTD CSS.

Part of the reason for this is that Alabaster and the RTD look quite different and the exact same ad placement may not work in both. We might want to use the light theme with Alabaster and the dark theme with RTD-like themes. I'm not sure.

[humitos]

I figured this out. I think it's working as expected. Here are some examples:

Theme	Image
Read the Docs
Alabaster
Docusaurus
Material for MkDocs

[humitos]

In my opinion, the next step here is to integrate the ad in a better way with other themes/tools. This is, for example, do not use stickybox in Material for MkDocs and place it in the left/right bar.

[humitos]

I added some extra code here to work on Docusaurus and Material for MkDocs in a better integrated way 😄

Theme	Image
Docusaurus
Material for MkDocs

[humitos]

IMO, this PR is ready to review and deploy. We can continue iterating and adding more cases in the following PRs.

[humitos]

Added another integration for Sphinx Book Theme (Jupyter Book)

[davidfischer]

Overall, this looks good.

As I mentioned at #295, we might be able to remove some of the isSphinxReadTheDocsLikeTheme()/isSphinxAlabasterLikeTheme() code by using the styling directly from the ad client instead of "readthedocs-sidebar" (just use data-ea-type="image", the default). Using the custom readthedocs-sidebar means you're responsible for the styling basically. I'm not sure this code can be removed 100% because you still might want to inject the ad in different places on different themes but you probably could just accept light/dark mode depending on the theme. However, what you have seems to look good although it does sort of mean RTD continues to maintain custom ad CSS. Perhaps switching to the built-in image placement in a v2?

[ericholscher]

As I mentioned at #295, we might be able to remove some of the isSphinxReadTheDocsLikeTheme()/isSphinxAlabasterLikeTheme() code by using the styling directly from the ad client instead of "readthedocs-sidebar" (just use data-ea-type="image", the default). Using the custom readthedocs-sidebar means you're responsible for the styling basically. I'm not sure this code can be removed 100% because you still might want to inject the ad in different places on different themes but you probably could just accept light/dark mode depending on the theme. However, what you have seems to look good although it does sort of mean RTD continues to maintain custom ad CSS. Perhaps switching to the built-in image placement in a v2?

Yea, if we could get away from custom RTD CSS that would be great. Hopefully the ad styles on the ad client should be enough for most things, but I agree we don't need to block this work on that, so we can merge this and then figure out deprecating the CSS styles.

[humitos]

I'm not opposed to move the CSS styles into the ad client and rely on light/dark mode only. However, we will still need some code in the addons (isReadTheDocsLikeTheme(), etc) to determine where to inject the ad based on the theme/doctool. I'm fine with that since we can give users a better experience than always using the stickybox.

[ericholscher]

@humitos yea, we definitely want logic around where to inject -- I think removing our own custom CSS is the next step 👍