From 228953725bba74aa9c7e8b01d2a01f0bb548867f Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 13 Jan 2025 16:16:12 +0100 Subject: [PATCH 1/7] Use Lit Context to share `config` across all addons * Reference: https://lit.dev/docs/data/context/ --- package-lock.json | 15 +++++++++++---- package.json | 3 ++- public/index.html | 5 +++++ src/context.js | 30 ++++++++++++++++++++++++++++++ src/flyout.js | 44 +++++++++++++++++++++----------------------- 5 files changed, 69 insertions(+), 28 deletions(-) create mode 100644 src/context.js diff --git a/package-lock.json b/package-lock.json index 2a28831d..9745ce01 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,8 @@ "version": "0.24.0", "license": "MIT", "dependencies": { - "@floating-ui/dom": "^1.6.13" + "@floating-ui/dom": "^1.6.13", + "@lit/context": "^1.1.3" }, "devDependencies": { "@babel/core": "^7.26.0", @@ -1813,14 +1814,20 @@ "node_modules/@lit-labs/ssr-dom-shim": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/@lit-labs/ssr-dom-shim/-/ssr-dom-shim-1.2.1.tgz", - "integrity": "sha512-wx4aBmgeGvFmOKucFKY+8VFJSYZxs9poN3SDNQFF6lT6NrQUnHiPB2PWz2sc4ieEcAaYYzN+1uWahEeTq2aRIQ==", - "dev": true + "integrity": "sha512-wx4aBmgeGvFmOKucFKY+8VFJSYZxs9poN3SDNQFF6lT6NrQUnHiPB2PWz2sc4ieEcAaYYzN+1uWahEeTq2aRIQ==" + }, + "node_modules/@lit/context": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/@lit/context/-/context-1.1.3.tgz", + "integrity": "sha512-Auh37F4S0PZM93HTDfZWs97mmzaQ7M3vnTc9YvxAGyP3UItSK/8Fs0vTOGT+njuvOwbKio/l8Cx/zWL4vkutpQ==", + "dependencies": { + "@lit/reactive-element": "^1.6.2 || ^2.0.0" + } }, "node_modules/@lit/reactive-element": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/@lit/reactive-element/-/reactive-element-2.0.4.tgz", "integrity": "sha512-GFn91inaUa2oHLak8awSIigYz0cU0Payr1rcFsrkf5OJ5eSPxElyZfKh0f2p9FsTiZWXQdWGJeXZICEfXXYSXQ==", - "dev": true, "dependencies": { "@lit-labs/ssr-dom-shim": "^1.2.0" } diff --git a/package.json b/package.json index 818bdf0a..5a438445 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ } }, "dependencies": { - "@floating-ui/dom": "^1.6.13" + "@floating-ui/dom": "^1.6.13", + "@lit/context": "^1.1.3" } } diff --git a/public/index.html b/public/index.html index f24ff41a..4905099e 100644 --- a/public/index.html +++ b/public/index.html @@ -22,6 +22,11 @@

Documentation Addons

CustomEvent

Project slug using CustomEvent:

+

Flyout

+ + + +

DocDiff

Visit this page to take a look at it.

diff --git a/src/context.js b/src/context.js new file mode 100644 index 00000000..abab9bcb --- /dev/null +++ b/src/context.js @@ -0,0 +1,30 @@ +import { html, nothing, render, LitElement } from "lit"; +import { ContextProvider } from "@lit/context"; +import { createContext } from "@lit/context"; +import { getReadTheDocsConfig } from "./readthedocs-config.js"; +import { EVENT_READTHEDOCS_ADDONS_DATA_READY } from "./events"; + +export const configContext = createContext(Symbol("readthedocs-config")); + +export class AddonsApp extends LitElement { + config = new ContextProvider(this, { context: configContext }); + + connectedCallback() { + super.connectedCallback(); + document.addEventListener( + EVENT_READTHEDOCS_ADDONS_DATA_READY, + this._handleAddonsDataReady, + ); + } + + createRenderRoot() { + return this; + } + + _handleAddonsDataReady = (event) => { + console.log("_handleAddonsDataReady"); + this.config.setValue(event.detail.data()); + }; +} + +customElements.define("readthedocs-context", AddonsApp); diff --git a/src/flyout.js b/src/flyout.js index 981893a0..e90ed1f4 100644 --- a/src/flyout.js +++ b/src/flyout.js @@ -1,11 +1,13 @@ import { ajv } from "./data-validation"; import READTHEDOCS_LOGO_WORDMARK from "./images/logo-wordmark-light.svg"; import READTHEDOCS_LOGO from "./images/logo-light.svg"; +import { ContextConsumer } from "@lit/context"; import { library, icon } from "@fortawesome/fontawesome-svg-core"; import { faCodeBranch, faLanguage } from "@fortawesome/free-solid-svg-icons"; import { html, nothing, render, LitElement } from "lit"; import { classMap } from "lit/directives/class-map.js"; import { default as objectPath } from "object-path"; +import { configContext } from "./context.js"; import styleSheet from "./flyout.css"; import { AddonBase, addUtmParameters, getLinkWithFilename } from "./utils"; @@ -19,7 +21,6 @@ export class FlyoutElement extends LitElement { static elementName = "readthedocs-flyout"; static properties = { - config: { state: true }, opened: { type: Boolean }, floating: { type: Boolean }, position: { type: String }, @@ -27,24 +28,24 @@ export class FlyoutElement extends LitElement { static styles = styleSheet; + // `_config` is the context we are going to consume when it's updated. + _config = new ContextConsumer(this, { + context: configContext, + subscribe: true, + }); + constructor() { super(); + // `config` is the internal config for this addon that's updated based on `_config`. this.config = null; + this.opened = false; this.floating = true; this.position = "bottom-right"; this.readthedocsLogo = READTHEDOCS_LOGO; - } - - loadConfig(config) { - // Validate the config object before assigning it to the Addon. - // Later, ``render()`` method will check whether this object exists and (not) render - // accordingly - if (!FlyoutAddon.isEnabled(config)) { - return; - } - this.config = config; + console.log("Flyout _config (from constructor() method)"); + console.log(this._config.value); } _close() { @@ -305,11 +306,17 @@ export class FlyoutElement extends LitElement { render() { // The element doesn't yet have our config, don't render it. - if (this.config === null) { - // nothing is a special Lit response type + console.log("Flyout config (from render() method )"); + console.log(this._config.value); + + // Validate the context (`this._config.value`) on each update and return + // nothing if it's invalid. ``nothing`` is a special Lit response type. + if (!FlyoutAddon.isEnabled(this._config.value)) { return nothing; } + this.config = this._config.value; + const classes = { floating: this.floating, container: true }; classes[this.position] = true; @@ -376,16 +383,7 @@ export class FlyoutAddon extends AddonBase { // If there are no elements found, inject one let elems = document.querySelectorAll("readthedocs-flyout"); if (!elems.length) { - elems = [new FlyoutElement()]; - - // We cannot use `render(elems[0], document.body)` because there is a race conditions between all the addons. - // So, we append the web-component first and then request an update of it. - document.body.append(elems[0]); - elems[0].requestUpdate(); - } - - for (const elem of elems) { - elem.loadConfig(config); + render(new FlyoutElement(), document.body); } } } From 1d340fd4f152bd027946ddd7547f50066b8fe747 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 13 Jan 2025 16:34:43 +0100 Subject: [PATCH 2/7] Migrate "NotificationAddon" to use Lit Context --- public/index.html | 16 ++++---- src/index.js | 1 + src/notification.js | 89 ++++++++++++++++++++------------------------- 3 files changed, 49 insertions(+), 57 deletions(-) diff --git a/public/index.html b/public/index.html index 4905099e..caf18bd9 100644 --- a/public/index.html +++ b/public/index.html @@ -24,7 +24,7 @@

CustomEvent

Flyout

- +

DocDiff

@@ -62,11 +62,13 @@

Link Previews

Notification

- - - - - - + + + + + + + + diff --git a/src/index.js b/src/index.js index b6827651..0416b638 100644 --- a/src/index.js +++ b/src/index.js @@ -74,6 +74,7 @@ export function setup() { if (addon.isEnabled(config, httpStatus)) { promises.push( new Promise((resolve) => { + // TODO: remove attribute `config` from here since it's not required anymore return resolve(new addon(config)); }), ); diff --git a/src/notification.js b/src/notification.js index 3bf491ea..e1533089 100644 --- a/src/notification.js +++ b/src/notification.js @@ -1,5 +1,6 @@ import { ajv } from "./data-validation"; import { library, icon } from "@fortawesome/fontawesome-svg-core"; +import { ContextConsumer } from "@lit/context"; import { faCircleXmark, faFlask, @@ -8,6 +9,7 @@ import { } from "@fortawesome/free-solid-svg-icons"; import { html, nothing, render, LitElement } from "lit"; import { default as objectPath } from "object-path"; +import { configContext } from "./context.js"; import styleSheet from "./notification.css"; import { AddonBase, addUtmParameters, getLinkWithFilename } from "./utils"; @@ -18,7 +20,6 @@ export class NotificationElement extends LitElement { /** @static @property {Object} - Lit reactive properties */ static properties = { - config: { state: true }, urls: { state: true }, highest_version: { state: true }, dismissedTimestamp: { state: true }, @@ -29,6 +30,12 @@ export class NotificationElement extends LitElement { /** @static @property {Object} - Lit stylesheets to apply to elements */ static styles = styleSheet; + // `_config` is the context we are going to consume when it's updated. + _config = new ContextConsumer(this, { + context: configContext, + subscribe: true, + }); + constructor() { super(); @@ -125,15 +132,32 @@ export class NotificationElement extends LitElement { this.timerID = null; } - loadConfig(config) { - // Validate the config object before assigning it to the Addon. - // Later, ``render()`` method will check whether this object exists and (not) render - // accordingly - if (!NotificationAddon.isEnabled(config)) { - return; + getLocalStorageKeyFromConfig(config) { + const projectSlug = config.projects.current.slug; + const languageCode = config.projects.current.language.code; + const versionSlug = config.versions.current.slug; + return `${projectSlug}-${languageCode}-${versionSlug}-notification`; + } + + firstUpdated() { + // Add CSS classes to the element on ``firstUpdated`` because we need the + // HTML element to exist in the DOM before being able to add tag attributes. + this.className = this.className || "raised toast"; + } + + render() { + if (this.autoDismissed) { + return nothing; } - this.config = config; + if (!NotificationAddon.isEnabled(this._config.value)) { + return nothing; + } + this.config = this._config.value; + + if (!this.config.addons.notifications.enabled) { + return nothing; + } if ( this.config.addons.notifications.enabled && @@ -142,11 +166,11 @@ export class NotificationElement extends LitElement { this.urls = { // NOTE: point users to the new beta dashboard for now so we promote it more. // We will revert this once we are fully migrated to the new dashboard. - build: config.builds.current.urls.build + build: this.config.builds.current.urls.build .replace("readthedocs.org", "app.readthedocs.org") .replace("readthedocs.com", "app.readthedocs.com") .replace("app.app.", "app."), - external: config.versions.current.urls.vcs, + external: this.config.versions.current.urls.vcs, }; } @@ -156,48 +180,19 @@ export class NotificationElement extends LitElement { "addons.notifications.show_on_latest", false, ) && - config.projects.current.versioning_scheme !== + this.config.projects.current.versioning_scheme !== "single_version_without_translations" && - config.versions.current.type !== "external" + this.config.versions.current.type !== "external" ) { this.calculateStableLatestVersionWarning(); } - this.loadDismissedTimestamp(this.config); - } - - getLocalStorageKeyFromConfig(config) { - const projectSlug = config.projects.current.slug; - const languageCode = config.projects.current.language.code; - const versionSlug = config.versions.current.slug; - return `${projectSlug}-${languageCode}-${versionSlug}-notification`; - } - - firstUpdated() { - // Add CSS classes to the element on ``firstUpdated`` because we need the - // HTML element to exist in the DOM before being able to add tag attributes. - this.className = this.className || "raised toast"; - } - - render() { - if (this.autoDismissed) { - return nothing; - } - - // The element doesn't yet have our config, don't render it. - if (this.config === null) { - // nothing is a special Lit response type - return nothing; - } + this.loadDismissedTimestamp(this.config); // This notification has been dimissed, so don't render it if (this.dismissedTimestamp) { return nothing; } - if (!this.config.addons.notifications.enabled) { - return nothing; - } - if (this.config.versions.current.type === "external") { if ( objectPath.get( @@ -411,19 +406,13 @@ export class NotificationAddon extends AddonBase { static addonEnabledPath = "addons.notifications.enabled"; static addonName = "Notification"; - constructor(config) { + constructor() { super(); // If there are no elements found, inject one let elems = document.querySelectorAll("readthedocs-notification"); if (!elems.length) { - elems = [new NotificationElement()]; - document.body.append(elems[0]); - elems[0].requestUpdate(); - } - - for (const elem of elems) { - elem.loadConfig(config); + render(new NotificationElement(), document.body); } } } From 4c01eb61e651cb5493052ea9d08d6f7b7bd7763f Mon Sep 17 00:00:00 2001 From: Anthony Date: Thu, 16 Jan 2025 03:23:19 -0800 Subject: [PATCH 3/7] Use ContextRoot at document.html instead (#499) Additional elements shouldn't be needed in our structure. This is how we can use ContextRoot with `document.html` as the context root for providers/consumers of the config context. Both the flyout and the notifications load for me with this change. --- public/index.html | 18 +++++++----------- src/context.js | 46 +++++++++++++++++++++------------------------- src/flyout.js | 9 +++++---- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/public/index.html b/public/index.html index caf18bd9..68016500 100644 --- a/public/index.html +++ b/public/index.html @@ -23,9 +23,7 @@

CustomEvent

Project slug using CustomEvent:

Flyout

- - - +

DocDiff

Visit this page to take a look at it.

@@ -62,13 +60,11 @@

Link Previews

Notification

- - - - - - - - + + + + + + diff --git a/src/context.js b/src/context.js index abab9bcb..587ce754 100644 --- a/src/context.js +++ b/src/context.js @@ -1,30 +1,26 @@ -import { html, nothing, render, LitElement } from "lit"; -import { ContextProvider } from "@lit/context"; -import { createContext } from "@lit/context"; -import { getReadTheDocsConfig } from "./readthedocs-config.js"; +import { + ContextProvider, + ContextRoot, + createContext, +} from "@lit/context"; import { EVENT_READTHEDOCS_ADDONS_DATA_READY } from "./events"; +export const contextRoot = new ContextRoot().attach(document.body); export const configContext = createContext(Symbol("readthedocs-config")); -export class AddonsApp extends LitElement { - config = new ContextProvider(this, { context: configContext }); +/** + * Because `config` provider is not attached to a ReactiveElement, and is + * instead connected to `document.html`, we have to call `hostConnected()` + * manually. See: + * + * https://github.com/lit/lit/blob/935697d47e62ed75e3157423400163a8371c62fc/packages/context/src/lib/controllers/context-provider.ts#L55-L58 + **/ +const config = new ContextProvider(document.documentElement, { + context: configContext, +}); +config.hostConnected(); - connectedCallback() { - super.connectedCallback(); - document.addEventListener( - EVENT_READTHEDOCS_ADDONS_DATA_READY, - this._handleAddonsDataReady, - ); - } - - createRenderRoot() { - return this; - } - - _handleAddonsDataReady = (event) => { - console.log("_handleAddonsDataReady"); - this.config.setValue(event.detail.data()); - }; -} - -customElements.define("readthedocs-context", AddonsApp); +document.addEventListener(EVENT_READTHEDOCS_ADDONS_DATA_READY, (event) => { + console.log("Event:", EVENT_READTHEDOCS_ADDONS_DATA_READY); + config.setValue(event.detail.data()); +}); diff --git a/src/flyout.js b/src/flyout.js index e90ed1f4..eb6fbaaf 100644 --- a/src/flyout.js +++ b/src/flyout.js @@ -44,8 +44,10 @@ export class FlyoutElement extends LitElement { this.floating = true; this.position = "bottom-right"; this.readthedocsLogo = READTHEDOCS_LOGO; - console.log("Flyout _config (from constructor() method)"); - console.log(this._config.value); + console.log( + "Flyout _config (from constructor() method)", + this._config.value, + ); } _close() { @@ -306,8 +308,7 @@ export class FlyoutElement extends LitElement { render() { // The element doesn't yet have our config, don't render it. - console.log("Flyout config (from render() method )"); - console.log(this._config.value); + console.log("Flyout config (from render() method )", this._config.value); // Validate the context (`this._config.value`) on each update and return // nothing if it's invalid. ``nothing`` is a special Lit response type. From 09cb9d03178e8b6e3b310e600197a4befbcfe634 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 16 Jan 2025 12:37:52 +0100 Subject: [PATCH 4/7] Migrate analytics to `LitElement` so we can use `ContextRoot` --- src/analytics.js | 76 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 20 deletions(-) diff --git a/src/analytics.js b/src/analytics.js index 447989a5..00be5e3a 100644 --- a/src/analytics.js +++ b/src/analytics.js @@ -5,36 +5,40 @@ import { default as fetch } from "unfetch"; import { ajv } from "./data-validation"; import { AddonBase } from "./utils"; import { CLIENT_VERSION } from "./utils"; +import { ContextConsumer } from "@lit/context"; +import { configContext } from "./context.js"; +import { nothing, render, LitElement } from "lit"; export const API_ENDPOINT = "/_/api/v2/analytics/"; -/** - * Analytics addon - * - * Register page views that can be seen from the project's dashboard. - * Besides, it injects the Global Read the Docs analytics. - * - * Read more at: - * - https://docs.readthedocs.io/en/stable/reference/analytics.html - * - https://docs.readthedocs.io/en/stable/advertising/advertising-details.html#analytics - * - * @param {Object} config - Addon configuration object - */ -export class AnalyticsAddon extends AddonBase { - static jsonValidationURI = - "http://v1.schemas.readthedocs.org/addons.analytics.json"; - static addonEnabledPath = "addons.analytics.enabled"; - static addonName = "Analytics"; - static enabledOnHttpStatus = [200, 404]; +export class AnalyticsElement extends LitElement { + static elementName = "readthedocs-analytics"; + + // `_config` is the context we are going to consume when it's updated. + _config = new ContextConsumer(this, { + context: configContext, + subscribe: true, + }); - constructor(config) { + constructor() { super(); - this.config = config; + this.config = null; + } + + render() { + // Validate the context (`this._config.value`) on each update and return + // nothing if it's invalid. ``nothing`` is a special Lit response type. + if (!AnalyticsAddon.isEnabled(this._config.value)) { + return nothing; + } + + this.config = this._config.value; // Only register pageviews on non-external versions if (this.config.versions.current.type !== "external") { this.registerPageView(); } + return nothing; } registerPageView() { @@ -59,3 +63,35 @@ export class AnalyticsAddon extends AddonBase { }); } } + +/** + * Analytics addon + * + * Register page views that can be seen from the project's dashboard. + * Besides, it injects the Global Read the Docs analytics. + * + * Read more at: + * - https://docs.readthedocs.io/en/stable/reference/analytics.html + * - https://docs.readthedocs.io/en/stable/advertising/advertising-details.html#analytics + * + * @param {Object} config - Addon configuration object + */ +export class AnalyticsAddon extends AddonBase { + static jsonValidationURI = + "http://v1.schemas.readthedocs.org/addons.analytics.json"; + static addonEnabledPath = "addons.analytics.enabled"; + static addonName = "Analytics"; + static enabledOnHttpStatus = [200, 404]; + + constructor() { + super(); + + // If there are no elements found, inject one + let elems = document.querySelectorAll("readthedocs-analytics"); + if (!elems.length) { + render(new AnalyticsElement(), document.body); + } + } +} + +customElements.define("readthedocs-analytics", AnalyticsElement); From 6988753a05f8c018ce7254336cea393aaa6aa326 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 16 Jan 2025 12:47:57 +0100 Subject: [PATCH 5/7] Migrate CustomScript to a Lit Element --- src/customscript.js | 62 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/src/customscript.js b/src/customscript.js index bdb594c3..f7d4d0b2 100644 --- a/src/customscript.js +++ b/src/customscript.js @@ -1,27 +1,38 @@ import { default as objectPath } from "object-path"; import { AddonBase } from "./utils"; +import { ContextConsumer } from "@lit/context"; +import { configContext } from "./context.js"; +import { nothing, render, LitElement } from "lit"; const SCRIPT_ID = "readthedocs-addons-custom-script"; -/** - * User JavaScript file. - * - * Allow a user to inject a custom JavaScript file in all the pages. - */ -export class CustomScriptAddon extends AddonBase { - static jsonValidationURI = - "http://v1.schemas.readthedocs.org/addons.customscript.json"; - static addonEnabledPath = "addons.customscript.enabled"; - static addonName = "CustomScript"; - static enabledOnHttpStatus = [200, 403, 404, 500]; +export class CustomScriptElement extends LitElement { + static elementName = "readthedocs-customscript"; + + // `_config` is the context we are going to consume when it's updated. + _config = new ContextConsumer(this, { + context: configContext, + subscribe: true, + }); - constructor(config) { + constructor() { super(); - this.config = config; + this.config = null; + } + + render() { + // Validate the context (`this._config.value`) on each update and return + // nothing if it's invalid. ``nothing`` is a special Lit response type. + if (!CustomScriptAddon.isEnabled(this._config.value)) { + return nothing; + } + + this.config = this._config.value; if (objectPath.get(this.config, "addons.customscript.src")) { this.injectJavaScriptFile(); } + return nothing; } injectJavaScriptFile() { @@ -33,3 +44,28 @@ export class CustomScriptAddon extends AddonBase { document.body.appendChild(script); } } + +/** + * User JavaScript file. + * + * Allow a user to inject a custom JavaScript file in all the pages. + */ +export class CustomScriptAddon extends AddonBase { + static jsonValidationURI = + "http://v1.schemas.readthedocs.org/addons.customscript.json"; + static addonEnabledPath = "addons.customscript.enabled"; + static addonName = "CustomScript"; + static enabledOnHttpStatus = [200, 403, 404, 500]; + + constructor() { + super(); + + // If there are no elements found, inject one + let elems = document.querySelectorAll("readthedocs-customscript"); + if (!elems.length) { + render(new CustomScriptElement(), document.body); + } + } +} + +customElements.define("readthedocs-customscript", CustomScriptElement); From 5f2e437f8ed549b9ceb822b399ab19e340cb6efc Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 16 Jan 2025 12:58:04 +0100 Subject: [PATCH 6/7] Use `ContextConsumer` for DocDiff --- public/docdiff.html | 1 + src/docdiff.js | 55 +++++++++++++++++---------------------------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/public/docdiff.html b/public/docdiff.html index 97ad35b4..547e8369 100644 --- a/public/docdiff.html +++ b/public/docdiff.html @@ -1,5 +1,6 @@ + DocDiff Addons - Read the Docs diff --git a/src/docdiff.js b/src/docdiff.js index ac2beef4..350e7bda 100644 --- a/src/docdiff.js +++ b/src/docdiff.js @@ -17,9 +17,11 @@ import { EVENT_READTHEDOCS_DOCDIFF_HIDE, EVENT_READTHEDOCS_ROOT_DOM_CHANGED, } from "./events"; -import { nothing, LitElement } from "lit"; +import { render, nothing, LitElement } from "lit"; import { default as objectPath } from "object-path"; import { hasQueryParam, docTool } from "./utils"; +import { ContextConsumer } from "@lit/context"; +import { configContext } from "./context.js"; export const DOCDIFF_URL_PARAM = "readthedocs-diff"; @@ -75,6 +77,12 @@ export class DocDiffElement extends LitElement { static styles = styleSheet; + // `_config` is the context we are going to consume when it's updated. + _config = new ContextConsumer(this, { + context: configContext, + subscribe: true, + }); + constructor() { super(); @@ -87,11 +95,7 @@ export class DocDiffElement extends LitElement { this.cachedRemoteContent = null; } - loadConfig(config) { - if (!DocDiffAddon.isEnabled(config)) { - return; - } - this.config = config; + firstUpdated() { this.rootSelector = objectPath.get(this.config, "options.root_selector") || docTool.getRootSelector(); @@ -101,7 +105,17 @@ export class DocDiffElement extends LitElement { if (this.injectStyles) { document.adoptedStyleSheets.push(docdiffGeneralStyleSheet); } + } + + render() { + // Validate the context (`this._config.value`) on each update and return + // nothing if it's invalid. ``nothing`` is a special Lit response type. + if (!DocDiffAddon.isEnabled(this._config.value)) { + return nothing; + } + console.log("DocDiff:", this._config.value); + this.config = this._config.value; // Enable DocDiff if the URL parameter is present if (hasQueryParam(DOCDIFF_URL_PARAM)) { const event = new CustomEvent( @@ -109,31 +123,10 @@ export class DocDiffElement extends LitElement { ); document.dispatchEvent(event); } - } - render() { return nothing; - // TODO: render a checkbox once we are settled on the UI. - // For now, we are only enabling/disabling via a hotkey. - // - // return html` - // - // `; } - // This code isn't used until we show a UI, - // and even then we'll want to trigger events to match state? - // handleClick(e) { - // if (e.target.checked) { - // this.enableDocDiff(); - // } else { - // this.disableDocDiff(); - // } - // } - compare() { let promiseData; @@ -264,13 +257,7 @@ export class DocDiffAddon extends AddonBase { customElements.define("readthedocs-docdiff", DocDiffElement); let elems = document.querySelectorAll("readthedocs-docdiff"); if (!elems.length) { - elems = [new DocDiffElement()]; - document.body.append(elems[0]); - elems[0].requestUpdate(); - } - - for (const elem of elems) { - elem.loadConfig(config); + render(new DocDiffElement(), document.body); } } From 1c3032bb9efef93073f9f95d62871a2c3d14022e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 16 Jan 2025 13:16:10 +0100 Subject: [PATCH 7/7] TODO comments --- src/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.js b/src/index.js index ac40869b..a86940db 100644 --- a/src/index.js +++ b/src/index.js @@ -102,6 +102,8 @@ export function setup() { } for (const addon of addons) { + // TODO: always initialize _all the addons_, they will be rendered or not based on the dynamic config context + // TODO: move the `httpStatus` usage for `isEnabled` into `render()` for each addon if (addon.isEnabled(config, httpStatus)) { promises.push( new Promise((resolve) => {