From 2701131aa7b820e3a88516cf9b179cee3e2116f7 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Thu, 26 Feb 2026 12:16:37 +0200 Subject: [PATCH 1/4] refactor(theme-controller): Document and simplify theme application logic - This commit cleans up some of the internal logic of the ThemingController, particularly around how themes are applied when context changes. It removes some redundant state management. - The commit also adds documentation of the ThemingController class and the exported factory function, explaining their purpose and usage. --- src/theming/theming-controller.ts | 175 +++++++++++++++++++++++------- 1 file changed, 138 insertions(+), 37 deletions(-) diff --git a/src/theming/theming-controller.ts b/src/theming/theming-controller.ts index 8a6d906ac..ea6f59046 100644 --- a/src/theming/theming-controller.ts +++ b/src/theming/theming-controller.ts @@ -20,7 +20,54 @@ import type { type ThemeProviderSource = 'uninitialized' | 'context' | 'global'; +/** + * A reactive controller that manages theme adoption for a Lit host element. + * + * It resolves the active theme from one of two sources, in order of priority: + * + * 1. **Context** — provided by an ancestor `` element via the Lit context API. + * 2. **Global** — the application-wide theme set via `configureTheme()`. + * + * When a context provider is present, the controller subscribes to its updates + * and stops listening to global theme change events. When the host element is + * disconnected and reconnected outside a provider, it automatically falls back + * to the global theme. + * + * Theme styles are applied directly to the host's shadow root via `adoptStyles` + * every time the active theme or variant changes. + * + * @example + * Basic usage — typically created via {@link addThemingController}: + * ```typescript + * import { addThemingController } from '../../theming/theming-controller.js'; + * import { all } from './themes/themes.js'; + * + * export default class IgcMyComponent extends LitElement { + * constructor() { + * super(); + * addThemingController(this, all); + * } + * } + * ``` + * + * @example + * Accessing the current theme and variant at runtime: + * ```typescript + * export default class IgcMyComponent extends LitElement { + * private readonly _themes = addThemingController(this, all); + * + * protected override render() { + * // Conditionally render based on active theme + * return this._themes.variant === 'dark' + * ? html`
` + * : html`
`; + * } + * } + * ``` + */ class ThemingController implements ReactiveController { + //#region Internal state + private readonly _host: ReactiveControllerHost & ReactiveElement; private readonly _themes: Themes; private readonly _options?: ThemingControllerConfig; @@ -29,22 +76,39 @@ class ThemingController implements ReactiveController { ReactiveElement >; - private _theme: Theme = 'bootstrap'; - private _variant: ThemeVariant = 'light'; + private _theme: Theme; + private _variant: ThemeVariant; private _themeSource: ThemeProviderSource = 'uninitialized'; + //#endregion + + //#region Public properties + + /** Gets the current theme. */ public get theme(): Theme { return this._theme; } + /** Gets the current theme variant. */ + public get variant(): ThemeVariant { + return this._variant; + } + + //#endregion + constructor( host: ReactiveControllerHost & ReactiveElement, themes: Themes, config?: ThemingControllerConfig ) { + const { theme, themeVariant } = getTheme(); + + this._theme = theme; + this._variant = themeVariant; this._host = host; this._themes = themes; this._options = config; + this._host.addController(this); this._contextConsumer = new ContextConsumer(this._host, { @@ -58,17 +122,16 @@ class ThemingController implements ReactiveController { }); } + //#region ReactiveController implementation + /** @internal */ public hostConnected(): void { // Check if we have a context value immediately when connected usually after the parent provider // is already in the DOM (i.e. creation of the component after initial render) const contextValue = this._contextConsumer.value; - - if (contextValue) { - this._applyContextTheme(contextValue); - } else { - this._applyGlobalTheme(); - } + contextValue + ? this._applyContextTheme(contextValue) + : this._applyGlobalTheme(); } /** @internal */ @@ -78,14 +141,21 @@ class ThemingController implements ReactiveController { } } + //#endregion + + //#region Event handling + /** @internal */ public handleEvent(): void { - // Only handle global theme change events if (this._themeSource === 'global') { this._applyGlobalTheme(); } } + //#endregion + + //#region Internal methods + private _applyContextTheme(contextValue: { theme: Theme; variant: ThemeVariant; @@ -96,12 +166,7 @@ class ThemingController implements ReactiveController { } this._themeSource = 'context'; - this._theme = contextValue.theme; - this._variant = contextValue.variant; - - this._adoptStyles(); - this._options?.themeChange?.call(this._host, this._theme); - this._host.requestUpdate(); + this._applyTheme(contextValue.theme, contextValue.variant); } private _applyGlobalTheme(): void { @@ -110,43 +175,79 @@ class ThemingController implements ReactiveController { _themeChangedEmitter.addEventListener(CHANGED_THEME_EVENT, this); } + const { theme, themeVariant } = getTheme(); + this._themeSource = 'global'; + this._applyTheme(theme, themeVariant); + } - const { theme: currentTheme, themeVariant } = getTheme(); - this._theme = currentTheme; - this._variant = themeVariant; + private _applyTheme(theme: Theme, variant: ThemeVariant): void { + this._theme = theme; + this._variant = variant; this._adoptStyles(); this._options?.themeChange?.call(this._host, this._theme); this._host.requestUpdate(); } - private _getStyles() { - const props = this._themes[this._variant]; - const styles = { shared: css``, theme: css`` }; - - for (const [name, sheet] of Object.entries(props)) { - if (name === 'shared') { - styles.shared = sheet; - } - if (name === this.theme) { - styles.theme = sheet; - } - } - - return styles; - } - - protected _adoptStyles(): void { + private _adoptStyles(): void { const ctor = this._host.constructor as typeof LitElement; - const { shared, theme } = this._getStyles(); + const shared = this._themes[this._variant].shared || css``; + const theme = this._themes[this._variant][this._theme] || css``; adoptStyles(this._host.shadowRoot!, [...ctor.elementStyles, shared, theme]); } + + //#endregion } /** - * Adds theming controller to the host component. + * Creates and attaches a {@link ThemingController} to the given host element. + * + * This is the preferred way to add theming support to a component. The controller + * is registered with the host's reactive controller lifecycle and automatically + * resolves the active theme from an ancestor `` context or + * falls back to the application-wide theme set via `configureTheme()`. + * + * @param host - The Lit element that will host the controller. + * @param themes - The theme styles map containing `light` and `dark` variant entries, + * each keyed by theme name (`bootstrap`, `material`, `fluent`, `indigo`) and + * an optional `shared` entry applied regardless of theme. + * @param config - Optional configuration. + * @param config.themeChange - Callback invoked on the host whenever the active theme changes. + * + * @returns The created {@link ThemingController} instance. + * + * @example + * Minimal setup in a component constructor: + * ```typescript + * import { addThemingController } from '../../theming/theming-controller.js'; + * import { all } from './themes/themes.js'; + * + * export default class IgcMyComponent extends LitElement { + * constructor() { + * super(); + * addThemingController(this, all); + * } + * } + * ``` + * + * @example + * With a `themeChange` callback and retained controller reference: + * ```typescript + * export default class IgcMyComponent extends LitElement { + * private readonly _themingController = addThemingController(this, all, { + * themeChange(theme) { + * // Called on `this` (the host) whenever the theme switches + * console.log(`Theme changed to: ${theme}`); + * }, + * }); + * + * protected override render() { + * return html`Current variant: ${this._themingController.variant}`; + * } + * } + * ``` */ export function addThemingController( host: ReactiveControllerHost & ReactiveElement, From 58721e09e09d3c187ddbc1c53d92b756d8b5a65e Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Thu, 26 Feb 2026 13:15:35 +0200 Subject: [PATCH 2/4] chore: Revert initialization of theme and variant --- src/theming/theming-controller.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/theming/theming-controller.ts b/src/theming/theming-controller.ts index ea6f59046..cb3d5ee81 100644 --- a/src/theming/theming-controller.ts +++ b/src/theming/theming-controller.ts @@ -76,8 +76,8 @@ class ThemingController implements ReactiveController { ReactiveElement >; - private _theme: Theme; - private _variant: ThemeVariant; + private _theme: Theme = 'bootstrap'; + private _variant: ThemeVariant = 'light'; private _themeSource: ThemeProviderSource = 'uninitialized'; //#endregion @@ -101,10 +101,6 @@ class ThemingController implements ReactiveController { themes: Themes, config?: ThemingControllerConfig ) { - const { theme, themeVariant } = getTheme(); - - this._theme = theme; - this._variant = themeVariant; this._host = host; this._themes = themes; this._options = config; From 18cd9d48b337d3f68ed761addf6dc38ae4ed99d0 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Thu, 26 Feb 2026 14:32:03 +0200 Subject: [PATCH 3/4] refactor(theme-controller): Document and simplify theme application logic Simplifies the theme provider and controller lifecycle management to improve reliability when components are dynamically added or moved in the DOM tree. Theme Provider Changes: - Move ContextProvider initialization from constructor to class field initializer - Remove constructor entirely (no longer needed) - Remove firstUpdated lifecycle method that redundantly set provider value - Consolidate theme/variant updates in the update() lifecycle method Theming Controller Changes: - Simplify hostConnected to always apply global theme first - Context providers synchronously invoke callbacks before first update cycle - This ensures correct theme resolution for dynamically created components - Add proper state cleanup in hostDisconnected: - Reset _themeSource to 'uninitialized' - Clear _contextConsumer value - Allows components to properly re-resolve theme when reconnected elsewhere - Improve inline documentation explaining the theme resolution logic Test Improvements: - Add comprehensive variant assertions throughout all test cases - Enhance component reconnection test with proper validation: - Tests moving component from inside provider to outside - Tests moving component back inside provider - Verifies theme source switches correctly in both directions These changes ensure that components correctly pick up theme context when: - Created inside an existing theme provider - Moved between different parts of the DOM tree - Moved in or out of theme provider scope --- .../theme-provider/theme-provider.ts | 19 ++--------- src/theming/theming-controller.spec.ts | 33 +++++++++++++++++-- src/theming/theming-controller.ts | 14 ++++---- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/components/theme-provider/theme-provider.ts b/src/components/theme-provider/theme-provider.ts index 8bdb33df6..c2e0e8d74 100644 --- a/src/components/theme-provider/theme-provider.ts +++ b/src/components/theme-provider/theme-provider.ts @@ -63,7 +63,9 @@ export default class IgcThemeProviderComponent extends LitElement { registerComponent(IgcThemeProviderComponent); } - private readonly _provider: ContextProvider; + private readonly _provider = new ContextProvider(this, { + context: themeContext, + }); /** * The theme to provide to descendant components. @@ -83,15 +85,6 @@ export default class IgcThemeProviderComponent extends LitElement { @property({ reflect: true }) public variant: ThemeVariant = 'light'; - constructor() { - super(); - - this._provider = new ContextProvider(this, { - context: themeContext, - initialValue: this._getContextValue(), - }); - } - protected override update(changedProperties: PropertyValues): void { if (changedProperties.has('theme') || changedProperties.has('variant')) { this._provider.setValue(this._getContextValue()); @@ -100,12 +93,6 @@ export default class IgcThemeProviderComponent extends LitElement { super.update(changedProperties); } - protected override firstUpdated(): void { - this.updateComplete.then(() => { - this._provider.setValue(this._getContextValue()); - }); - } - private _getContextValue(): ThemeContext { return { theme: this.theme, diff --git a/src/theming/theming-controller.spec.ts b/src/theming/theming-controller.spec.ts index 280270135..dacb27889 100644 --- a/src/theming/theming-controller.spec.ts +++ b/src/theming/theming-controller.spec.ts @@ -159,6 +159,7 @@ describe('Theming Controller', () => { ); expect(el.themingController.theme).to.equal('bootstrap'); + expect(el.themingController.variant).to.equal('light'); }); it('should respond to global theme changes', async () => { @@ -168,6 +169,7 @@ describe('Theming Controller', () => { ); expect(el.themingController.theme).to.equal('bootstrap'); + expect(el.themingController.variant).to.equal('light'); // Change global theme setTimeout(() => configureTheme('material', 'light')); @@ -175,6 +177,7 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal('material'); + expect(el.themingController.variant).to.equal('light'); }); it('should call themeChange callback when global theme changes', async () => { @@ -192,6 +195,7 @@ describe('Theming Controller', () => { expect(el.themeChangeCallCount).to.be.greaterThan(initialCallCount); expect(el.lastTheme).to.equal('fluent'); + expect(el.themingController.variant).to.equal('dark'); }); it('should work without themeChange callback', async () => { @@ -201,6 +205,7 @@ describe('Theming Controller', () => { ); expect(el.themingController.theme).to.equal('bootstrap'); + expect(el.themingController.variant).to.equal('light'); // Should not throw when changing theme setTimeout(() => configureTheme('indigo', 'light')); @@ -208,6 +213,7 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal('indigo'); + expect(el.themingController.variant).to.equal('light'); }); it('should stop listening to global events when disconnected', async () => { @@ -247,6 +253,7 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal('material'); + expect(el.themingController.variant).to.equal('dark'); }); it('should update when theme provider theme changes', async () => { @@ -268,12 +275,14 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal('bootstrap'); + expect(el.themingController.variant).to.equal('light'); provider.theme = 'fluent'; await elementUpdated(provider); await elementUpdated(el); expect(el.themingController.theme).to.equal('fluent'); + expect(el.themingController.variant).to.equal('light'); }); it('should update when theme provider variant changes', async () => { @@ -321,6 +330,7 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal('material'); + expect(el.themingController.variant).to.equal('dark'); // Change global theme - should not affect component inside provider setTimeout(() => configureTheme('bootstrap', 'light')); @@ -329,6 +339,7 @@ describe('Theming Controller', () => { // Should still be material, not bootstrap expect(el.themingController.theme).to.equal('material'); + expect(el.themingController.variant).to.equal('dark'); }); it('should call themeChange callback when context theme changes', async () => { @@ -357,6 +368,7 @@ describe('Theming Controller', () => { expect(el.themeChangeCallCount).to.be.greaterThan(initialCallCount); expect(el.lastTheme).to.equal('indigo'); + expect(el.themingController.variant).to.equal('light'); }); it('should use nearest theme provider when nested', async () => { @@ -383,7 +395,9 @@ describe('Theming Controller', () => { await elementUpdated(innerEl); expect(outerEl.themingController.theme).to.equal('material'); + expect(outerEl.themingController.variant).to.equal('light'); expect(innerEl.themingController.theme).to.equal('fluent'); + expect(innerEl.themingController.variant).to.equal('dark'); }); }); @@ -409,6 +423,7 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal('indigo'); + expect(el.themingController.variant).to.equal('dark'); }); it('should fall back to global theme when moved outside provider', async () => { @@ -427,17 +442,28 @@ describe('Theming Controller', () => { const el = container.querySelector( themedTag ) as ThemedTestComponentElement; + const provider = container.querySelector( + IgcThemeProviderComponent.tagName + )!; await elementUpdated(el); + // After initial render, should have provider theme expect(el.themingController.theme).to.equal('material'); + expect(el.themingController.variant).to.equal('dark'); // Move outside the provider container.appendChild(el); await elementUpdated(el); - // Note: The component should now use global theme - // However, the context consumer may still hold the previous value - // This tests the disconnect/reconnect behavior + expect(el.themingController.theme).to.equal('bootstrap'); + expect(el.themingController.variant).to.equal('light'); + + // Move back inside provider scope + provider.appendChild(el); + await elementUpdated(el); + + expect(el.themingController.theme).to.equal('material'); + expect(el.themingController.variant).to.equal('dark'); }); }); @@ -503,6 +529,7 @@ describe('Theming Controller', () => { await elementUpdated(el); expect(el.themingController.theme).to.equal(theme); + expect(el.themingController.variant).to.equal(variant); }); } } diff --git a/src/theming/theming-controller.ts b/src/theming/theming-controller.ts index cb3d5ee81..a8f4638f8 100644 --- a/src/theming/theming-controller.ts +++ b/src/theming/theming-controller.ts @@ -122,12 +122,9 @@ class ThemingController implements ReactiveController { /** @internal */ public hostConnected(): void { - // Check if we have a context value immediately when connected usually after the parent provider - // is already in the DOM (i.e. creation of the component after initial render) - const contextValue = this._contextConsumer.value; - contextValue - ? this._applyContextTheme(contextValue) - : this._applyGlobalTheme(); + // Apply the global theme; if a context provider is present, the callback will be synchronously invoked + // and override it with the context value before the first update cycle. + this._applyGlobalTheme(); } /** @internal */ @@ -135,6 +132,11 @@ class ThemingController implements ReactiveController { if (this._themeSource === 'global') { _themeChangedEmitter.removeEventListener(CHANGED_THEME_EVENT, this); } + + // Reset to initial state so that if the host reconnects in a different part of the tree, + // it can properly re-resolve the theme source. + this._themeSource = 'uninitialized'; + this._contextConsumer.value = undefined; } //#endregion From 7577ea16f32709effdec0138e63aa896cdbcb47c Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Thu, 26 Feb 2026 14:42:42 +0200 Subject: [PATCH 4/4] Update src/theming/theming-controller.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/theming/theming-controller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/theming/theming-controller.ts b/src/theming/theming-controller.ts index a8f4638f8..406d0d17e 100644 --- a/src/theming/theming-controller.ts +++ b/src/theming/theming-controller.ts @@ -122,8 +122,8 @@ class ThemingController implements ReactiveController { /** @internal */ public hostConnected(): void { - // Apply the global theme; if a context provider is present, the callback will be synchronously invoked - // and override it with the context value before the first update cycle. + // Apply the global theme initially. When the host's first update cycle begins, if a context provider is + // present and has set its value, the callback will be invoked and override with the context value. this._applyGlobalTheme(); }