From b63190989d55baf693b1dcf834859be77b4b23e3 Mon Sep 17 00:00:00 2001 From: Josh Howenstine Date: Thu, 12 Sep 2024 05:30:41 -0700 Subject: [PATCH 1/5] fix: make sure subThemes are taken into account when generating style cache --- .../ui-components/src/globals/context/theme-manager.js | 6 +++++- .../src/mixins/withThemeStyles/StyleManager.js | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js b/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js index c6572f59a..4cd573fdd 100644 --- a/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js +++ b/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js @@ -207,7 +207,11 @@ class ThemeManager { return; } const globalTheme = this.getTheme(); - const subTheme = this._processTheme.call(this, [globalTheme, value]); + const subTheme = this._processTheme.call( + this, + [globalTheme, value], + value.extensions // Need a force option? + ); if (subTheme.font && subTheme.font.length) { await this._loadFonts(subTheme.font); } diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js index 11251a837..15eed4a8d 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js @@ -127,6 +127,7 @@ export default class StyleManager extends lng.EventEmitter { _generateCacheKey(name) { const cacheKey = [ name, + this.component._targetSubTheme, this.component.constructor.__componentName, this._customStyleHash ] From 3a3755834a176ed5054aa1408dac822361ba6b7f Mon Sep 17 00:00:00 2001 From: Josh Howenstine Date: Tue, 17 Sep 2024 10:33:21 -0700 Subject: [PATCH 2/5] fix: make sure style chain gets updated on subThemes --- .../mixins/withThemeStyles/StyleManager.js | 6 ++++- .../src/mixins/withThemeStyles/index.js | 1 + .../src/mixins/withThemeStyles/utils.js | 9 ++++--- .../.storybook/utils/FocusRing.styles.js | 26 +++++++++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js index 15eed4a8d..1b74560ae 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js @@ -100,6 +100,10 @@ export default class StyleManager extends lng.EventEmitter { this.update(); } + clearStyleChainCache() { + clearStyleChainCache(); + } + /** * Clears the source cache. */ @@ -205,7 +209,7 @@ export default class StyleManager extends lng.EventEmitter { // Style source does not exist so it will need to be generated. We attempt to run this function only when necessary for optimal performance styleSource = generateComponentStyleSource({ alias: this.component.constructor.aliasStyles, - componentConfig: this.component._componentConfig, + // componentConfig: this.component._componentConfig, inlineStyle: this.component._componentLevelStyle, styleChain: getStyleChainMemoized(this.component), theme: this.component.theme diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/index.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/index.js index 6fd500d82..97a8b769e 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/index.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/index.js @@ -120,6 +120,7 @@ export default function withThemeStyles(Base, mixinStyle = {}) { if (this._targetSubTheme) { this._styleManager.clearListeners(); this._styleManager.setupListeners(); + this._styleManager.clearStyleChainCache(); this._styleManager.clearStyleCache(); this._styleManager.clearSourceCache(); this._styleManager.update(); diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js index 176c3a752..b721b84af 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js @@ -641,9 +641,9 @@ export const getStyleChainMemoized = componentObj => { const cacheKey = generateNameFromPrototypeChain(componentObj); // Check if the result is already in the cache - if (styleChainCache[cacheKey]) { - return styleChainCache[cacheKey]; - } + // if (styleChainCache[cacheKey]) { + // return styleChainCache[cacheKey]; + // } /** * Compute the style chain using the getStyleChain function. @@ -675,6 +675,9 @@ export const getStyleChain = componentObj => { typeof proto === 'object' && proto.hasOwnProperty('constructor') ) { + if (proto.constructor.name === 'Tab') { + console.log(proto.title); + } // ComponentConfig Level const { style: componentConfigStyle } = getComponentConfig(proto); if (Object.keys(componentConfigStyle || {}).length) { diff --git a/packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js b/packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js new file mode 100644 index 000000000..084a713f3 --- /dev/null +++ b/packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js @@ -0,0 +1,26 @@ +export const base = theme => ({ + animationDuration: theme.animation.duration.xslow * 4, + borderWidth: theme.stroke.md, + colorTransitionAlpha: theme.alpha.secondary, + radius: theme.radius.md, + spacing: theme.spacer.xs, + shouldAnimate: true +}); + +export const tone = theme => ({ + neutral: { + color: theme.color.interactiveNeutralFocus, + transitionColor: null, + secondaryColor: theme.color.interactiveNeutralFocusSoft + }, + inverse: { + color: theme.color.interactiveInverseFocus, + transitionColor: null, + secondaryColor: theme.color.interactiveInverseFocusSoft + }, + brand: { + color: theme.color.interactiveBrandFocus, + transitionColor: null, + secondaryColor: theme.color.interactiveBrandFocusSoft + } +}); From 88d523433fe147ac95cea155134766a1928c2e1d Mon Sep 17 00:00:00 2001 From: Josh Howenstine Date: Tue, 17 Sep 2024 10:48:55 -0700 Subject: [PATCH 3/5] fix: cleanup comments and test file --- .../mixins/withThemeStyles/StyleManager.js | 1 - .../src/mixins/withThemeStyles/utils.js | 6 ++--- .../.storybook/utils/FocusRing.styles.js | 26 ------------------- 3 files changed, 3 insertions(+), 30 deletions(-) delete mode 100644 packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js index 1b74560ae..b33f8437b 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js @@ -209,7 +209,6 @@ export default class StyleManager extends lng.EventEmitter { // Style source does not exist so it will need to be generated. We attempt to run this function only when necessary for optimal performance styleSource = generateComponentStyleSource({ alias: this.component.constructor.aliasStyles, - // componentConfig: this.component._componentConfig, inlineStyle: this.component._componentLevelStyle, styleChain: getStyleChainMemoized(this.component), theme: this.component.theme diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js index b721b84af..f9d14cdff 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js @@ -641,9 +641,9 @@ export const getStyleChainMemoized = componentObj => { const cacheKey = generateNameFromPrototypeChain(componentObj); // Check if the result is already in the cache - // if (styleChainCache[cacheKey]) { - // return styleChainCache[cacheKey]; - // } + if (styleChainCache[cacheKey]) { + return styleChainCache[cacheKey]; + } /** * Compute the style chain using the getStyleChain function. diff --git a/packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js b/packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js deleted file mode 100644 index 084a713f3..000000000 --- a/packages/apps/lightning-ui-docs/.storybook/utils/FocusRing.styles.js +++ /dev/null @@ -1,26 +0,0 @@ -export const base = theme => ({ - animationDuration: theme.animation.duration.xslow * 4, - borderWidth: theme.stroke.md, - colorTransitionAlpha: theme.alpha.secondary, - radius: theme.radius.md, - spacing: theme.spacer.xs, - shouldAnimate: true -}); - -export const tone = theme => ({ - neutral: { - color: theme.color.interactiveNeutralFocus, - transitionColor: null, - secondaryColor: theme.color.interactiveNeutralFocusSoft - }, - inverse: { - color: theme.color.interactiveInverseFocus, - transitionColor: null, - secondaryColor: theme.color.interactiveInverseFocusSoft - }, - brand: { - color: theme.color.interactiveBrandFocus, - transitionColor: null, - secondaryColor: theme.color.interactiveBrandFocusSoft - } -}); From 8d6d42a288dd30dafa54153a65100a2aca24d32a Mon Sep 17 00:00:00 2001 From: Josh Howenstine Date: Tue, 17 Sep 2024 10:52:58 -0700 Subject: [PATCH 4/5] fix: cleanup comments and test file --- .../ui-components/src/mixins/withThemeStyles/StyleManager.js | 3 +++ .../ui-components/src/mixins/withThemeStyles/utils.js | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js index b33f8437b..55f7155e3 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/StyleManager.js @@ -100,6 +100,9 @@ export default class StyleManager extends lng.EventEmitter { this.update(); } + /** + * Clears the style chain cache. + */ clearStyleChainCache() { clearStyleChainCache(); } diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js index f9d14cdff..176c3a752 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js @@ -675,9 +675,6 @@ export const getStyleChain = componentObj => { typeof proto === 'object' && proto.hasOwnProperty('constructor') ) { - if (proto.constructor.name === 'Tab') { - console.log(proto.title); - } // ComponentConfig Level const { style: componentConfigStyle } = getComponentConfig(proto); if (Object.keys(componentConfigStyle || {}).length) { From 25bf7839634596da75142bfea117f693f4976f92 Mon Sep 17 00:00:00 2001 From: Josh Howenstine Date: Wed, 18 Sep 2024 08:10:48 -0700 Subject: [PATCH 5/5] fix: revert theme manager changes --- packages/@lightningjs/ui-components/package.json | 2 +- .../ui-components/src/globals/context/theme-manager.js | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/@lightningjs/ui-components/package.json b/packages/@lightningjs/ui-components/package.json index b6f03bb44..fc0b2f9be 100644 --- a/packages/@lightningjs/ui-components/package.json +++ b/packages/@lightningjs/ui-components/package.json @@ -58,4 +58,4 @@ "publishConfig": { "access": "public" } -} +} \ No newline at end of file diff --git a/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js b/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js index 4cd573fdd..c6572f59a 100644 --- a/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js +++ b/packages/@lightningjs/ui-components/src/globals/context/theme-manager.js @@ -207,11 +207,7 @@ class ThemeManager { return; } const globalTheme = this.getTheme(); - const subTheme = this._processTheme.call( - this, - [globalTheme, value], - value.extensions // Need a force option? - ); + const subTheme = this._processTheme.call(this, [globalTheme, value]); if (subTheme.font && subTheme.font.length) { await this._loadFonts(subTheme.font); }