EthicalAds: split .ethical-alabaster CSS class into .ethical-light-theme

[humitos]

We should not be using .ethical-alabaster for documentation tools that are not Sphinx and are not Alabaster theme. However, as they were looking better than the default style, we started used them there.

We should create a new CSS class for these cases.

• Bring CSS styles from Read the Docs addons into ad client ethical-ad-client#193
• EthicalAd: specific placement for Antora and mdBook #475 (comment)

[ericholscher]

I looked at this briefly, and all the styling seems to be specific to Alasbaster? Seems like we should probably keep this named alabaster, and not apply it to other themes?

addons/src/ethicalads.css

Lines 204 to 219 in e7d1d5f

	/* Alabaster specific customizations */
	.ethical-alabaster a.ethical-image-link {
	/* Alabaster adds a border even to image links on hover */
	border: 0 !important;
	}
	.ethical-alabaster hr {
	/* Alabaster needs some extra spacing before the footer ad */
	margin-top: 2em;
	}
	.ethical-alabaster::before {
	/* Alabaster's search box above the ad is floating */
	clear: both;
	content: "";
	display: table;
	margin-top: 3em;
	}

[humitos]

These rules work fine with Sphinx light themes, where we are already using it, that's why the rename was proposed. It seems it's not alabaster-specific. We can keep the .ethical-alabaster if we want and create another class; but we will be copying the same rules.

[ericholscher]

But it seems like it's adding padding and adjusting the page around the alabaster theme, not anything to do with the color?

[humitos]

Right, but we are using that class with those rules for other themes than alabaster. Even, for other documentation tools that are not Sphinx:

addons/src/ethicalads.js

Lines 88 to 191 in 398478a

	} else if (docTool.isSphinxFuroLikeTheme()) {
	// NOTE: The code to handle furo theme shouldn't be required,
	// since furo uses explicit placement.
	// However, the Jinja context variable READTHEDOCS is not injected anymore,
	// and furo does not includes the explicit placement due to this.
	// This is a temporal solution while they fix this.
	selector = ".sidebar-tree";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	placement.setAttribute("id", "furo-sidebar-ad-placement");
	knownPlacementFound = true;
	}
	} else if (docTool.isSphinxBookThemeLikeTheme()) {
	selector = ".sidebar-primary-items__start.sidebar-primary__section";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	knownPlacementFound = true;
	}
	} else if (docTool.isSphinxAlabasterLikeTheme()) {
	selector = "div.sphinxsidebar > div.sphinxsidebarwrapper";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	knownPlacementFound = true;
	}
	} else if (docTool.isMaterialMkDocsTheme()) {
	selector = ".md-sidebar__scrollwrap";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	// TODO: use a more styled CSS class.
	// See https://github.com/readthedocs/ethical-ad-client/issues/193
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	knownPlacementFound = true;
	}
	} else if (docTool.isDocusaurusTheme()) {
	selector = ".menu.thin-scrollbar.menu_SIkG";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	// TODO: use a more styled CSS class.
	// See https://github.com/readthedocs/ethical-ad-client/issues/193
	placement.classList.add("ethical-alabaster");
	placement.classList.add("ethical-docusaurus");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	placement.setAttribute("data-ea-style", "image");
	knownPlacementFound = true;
	}
	} else if (docTool.isDocsify()) {
	selector = "main > aside > div.sidebar-nav";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.classList.add("ethical-docsify");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	placement.setAttribute("data-ea-style", "image");
	knownPlacementFound = true;
	}
	} else if (docTool.isAntora()) {
	selector = "aside nav.nav-menu";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	placement.setAttribute("data-ea-style", "image");
	knownPlacementFound = true;
	}
	} else if (docTool.isMdBook()) {
	selector = "nav#sidebar mdbook-sidebar-scrollbox";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	placement.setAttribute("data-ea-style", "image");
	knownPlacementFound = true;
	}
	} else if (docTool.isVitePress()) {
	selector = "aside";
	element = document.querySelector(selector);
	if (this.elementAboveTheFold(element)) {
	placement.classList.add("ethical-alabaster");
	placement.setAttribute("data-ea-type", "readthedocs-sidebar");
	placement.setAttribute("data-ea-style", "image");
	knownPlacementFound = true;
	}

[ericholscher]

Right, but I'm saying that it doesn't make sense to apply that class there? Is it actually doing anything, since the structure of the content isn't the same?

I'm a little confused what the goal here is. Are we trying to make a class that applies different changes to light-themed docs that aren't already implemented?

[humitos]

Is it actually doing anything, since the structure of the content isn't the same?

Have you tried removing that class and showing an ad on pages from those documentation tools?

I'm a little confused what the goal here is.

The goal is to no use a class called -alabaster on themes that are not alabaster. If all those themes where we are currently using -alabaster are light themes, the new CSS class could be called -light-theme. On the other hand, if that's not true for all the current usages, we can use different class names.