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.

Sign up for GitHub

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

More variable fixes #487

Merged
merged 3 commits into from
Jan 14, 2025
+42 −37
Merged

More variable fixes #487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe1319f
Show example of setting attributes and variables at parent DOM
agjohnson Jan 8, 2025
6b5db39
More fixes
agjohnson Jan 9, 2025
12f1b3f
Update font sizes
agjohnson Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 28 additions & 20 deletions src/doctools.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
/* Specific styles based on documentation tools and themes */
html[data-readthedocs-tool="mkdocs-material"] {
--readthedocs-font-size: 0.58rem;
--readthedocs-flyout-font-size: 0.58rem;
}
/*
* Specific styles based on documentation tools and themes
*
* Usage of `@layer` at-rule pushes this rules down a step in
* precedence/priority. This allows a user `:root` rule to override these
* values.
**/
@layer defaults {
:root[data-readthedocs-tool="mkdocs-material"] {
--readthedocs-font-size: 0.58rem;
--readthedocs-flyout-font-size: 0.58rem;
}

html[data-readthedocs-tool="antora"] {
--readthedocs-font-size: 0.7rem;
--readthedocs-flyout-font-size: 0.7rem;
}
:root[data-readthedocs-tool="antora"] {
--readthedocs-font-size: 0.7rem;
--readthedocs-flyout-font-size: 0.7rem;
}

html[data-readthedocs-tool="mdbook"] {
--readthedocs-font-size: 1.3rem;
--readthedocs-flyout-font-size: 1.3rem;
}
:root[data-readthedocs-tool="mdbook"] {
--readthedocs-font-size: 1.3rem;
--readthedocs-flyout-font-size: 1.3rem;
}

html[data-readthedocs-tool="sphinx"][data-readthedocs-tool-theme="furo"] {
--readthedocs-font-size: 0.75rem;
--readthedocs-flyout-font-size: 0.75rem;
}
:root[data-readthedocs-tool="sphinx"][data-readthedocs-tool-theme="furo"] {
--readthedocs-font-size: 0.75rem;
--readthedocs-flyout-font-size: 0.75rem;
}

html[data-readthedocs-tool="sphinx"][data-readthedocs-tool-theme="immaterial"] {
--readthedocs-font-size: 0.58rem;
--readthedocs-flyout-font-size: 0.58rem;
:root[data-readthedocs-tool="sphinx"][data-readthedocs-tool-theme="immaterial"] {
--readthedocs-font-size: 0.58rem;
--readthedocs-flyout-font-size: 0.58rem;
}
}
12 changes: 6 additions & 6 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { CSSResult } from "lit";

import { getReadTheDocsConfig } from "./readthedocs-config";
import * as notification from "./notification";
import * as analytics from "./analytics";
Expand All @@ -15,7 +17,6 @@ import {
domReady,
isEmbedded,
IS_PRODUCTION,
IS_TESTING,
setupLogging,
getMetadataValue,
} from "./utils";
Expand Down Expand Up @@ -58,12 +59,11 @@ export function setup() {
const elementHtml = document.querySelector("html");
if (elementHtml) {
// Inject styles at the parent DOM to set variables at :root
//
// NOTE: We can't include this on testing due to this error:
// Failed to set the 'adoptedStyleSheets' property on 'Document': Failed to convert value to 'CSSStyleSheet'.
if (!IS_TESTING) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have this pattern in a couple other places that might be worth removing as well 💯

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to follow this path, we should probably create a helper function that does this correctly for us so we don't have to include this block of code everywhere.

By the way, @agjohnson do you understand why this doesn't work on tests? Why the doctoolsStyleSheet is not an instance of CSSResult under testing? 🤷🏼

Copy link
Contributor Author

@agjohnson agjohnson Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to change this pattern elsewhere yet. I'm just fixing the import for our tests rather than masking this code from tests. I didn't handle the test failure in my last PR, this conditional was added afterwards.

The error comes from web-test-runner using Rollup and our builds using Webpack. Both these handle the import { styleSheet } from "flyout.css" by exporting differing classed objects.

document.adoptedStyleSheets = [doctoolsStyleSheet];
let styleSheet = doctoolsStyleSheet;
if (doctoolsStyleSheet instanceof CSSResult) {
styleSheet = doctoolsStyleSheet.styleSheet;
}
document.adoptedStyleSheets = [styleSheet];

// If we detect a documentation tool, set attributes on :root to allow
// for CSS selectors to utilize these values.
Expand Down
19 changes: 8 additions & 11 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,17 +464,14 @@ export class DocumentationTool {
// <!-- Book generated using mdBook -->
// <meta charset="UTF-8">
// ...
try {
if (
document.head.firstChild.nextSibling.textContent.includes(
"Book generated using mdBook",
)
) {
return true;
}
} finally {
return false;
if (
document?.head?.firstChild?.nextSibling?.textContent.includes(
"Book generated using mdBook",
)
) {
return true;
}
return false;
}

isDocsify() {
Expand Down Expand Up @@ -539,7 +536,7 @@ export class DocumentationTool {
// MkDocs version : 1.4.2
// Build Date UTC : 2023-07-11 16:08:07.379780+00:00
// -->
if (document.lastChild.textContent.includes("MkDocs version :")) {
if (document?.lastChild?.textContent.includes("MkDocs version :")) {
return true;
}
return false;
Expand Down