From ca3d2d1bb7a5676832dd1ff6eda76c1d9567dfe0 Mon Sep 17 00:00:00 2001 From: Tim Hojnoski Date: Wed, 20 Dec 2023 14:22:16 -0500 Subject: [PATCH] Revert "fix: update global typography props when new theme is set (#432)" (#442) This reverts commit 28d3f954ae2267c582330ec0b59149a21fe53818. Co-authored-by: Emily Rautenberg --- .../src/components/Card/Card.test.js | 10 ++-- .../src/components/Card/CardRadio.test.js | 7 +-- .../src/components/Card/CardTitle.test.js | 27 +++++---- .../InlineContent/InlineContent.test.js | 2 +- .../src/components/Input/Input.js | 2 +- .../src/components/Input/Input.styles.js | 4 +- .../Input/__snapshots__/Input.test.js.snap | 2 +- .../__snapshots__/Keyboard.test.js.snap | 2 +- .../MetadataCardContent.test.js.snap | 1 + .../__snapshots__/Provider.test.js.snap | 1 + .../__snapshots__/ScrollWrapper.test.js.snap | 4 ++ .../src/mixins/withThemeStyles/utils.js | 59 ++----------------- .../src/mixins/withThemeStyles/utils.test.js | 36 +++-------- 13 files changed, 49 insertions(+), 108 deletions(-) diff --git a/packages/@lightningjs/ui-components/src/components/Card/Card.test.js b/packages/@lightningjs/ui-components/src/components/Card/Card.test.js index 431757982..dfe8cd89c 100644 --- a/packages/@lightningjs/ui-components/src/components/Card/Card.test.js +++ b/packages/@lightningjs/ui-components/src/components/Card/Card.test.js @@ -64,10 +64,12 @@ describe('Card', () => { card.title = 'Title'; testRenderer.forceAllUpdates(); expect(card._Title.content).toEqual('Title'); - expect(card._Title.style.textStyle).toMatchObject({ - ...card.style.titleTextStyle, - wordWrapWidth: card._calculateTextWidth() - }); + expect(card._Title.style.textStyle).toMatchObject( + card.style.titleTextStyle + ); + expect(card._Title.style.textStyle.wordWrapWidth).toEqual( + card._calculateTextWidth() + ); expect(card._Title.style.textStyle.textColor).toEqual( card.style.titleTextStyle.textColor ); diff --git a/packages/@lightningjs/ui-components/src/components/Card/CardRadio.test.js b/packages/@lightningjs/ui-components/src/components/Card/CardRadio.test.js index 6e85869df..a1184eeb5 100644 --- a/packages/@lightningjs/ui-components/src/components/Card/CardRadio.test.js +++ b/packages/@lightningjs/ui-components/src/components/Card/CardRadio.test.js @@ -33,10 +33,9 @@ describe('CardRadio', () => { cardRadio.subtitle = 'subtitle'; testRenderer.forceAllUpdates(); expect(cardRadio._Subtitle.content).toEqual('subtitle'); - expect(cardRadio._Subtitle.style.textStyle).toMatchObject({ - ...cardRadio.style.subtitleTextStyle, - wordWrapWidth: cardRadio._calculateTextWidth() - }); + expect(cardRadio._Subtitle.style.textStyle).toMatchObject( + cardRadio.style.subtitleTextStyle + ); }); it('should update subtitle position', () => { diff --git a/packages/@lightningjs/ui-components/src/components/Card/CardTitle.test.js b/packages/@lightningjs/ui-components/src/components/Card/CardTitle.test.js index c2e34b3ee..ce9775136 100644 --- a/packages/@lightningjs/ui-components/src/components/Card/CardTitle.test.js +++ b/packages/@lightningjs/ui-components/src/components/Card/CardTitle.test.js @@ -68,11 +68,17 @@ describe('CardTitle', () => { cardTitle.description = 'Description'; testRenderer.forceAllUpdates(); expect(cardTitle._Description.content).toEqual('Description'); - expect(cardTitle._Description.style.textStyle).toMatchObject({ - ...cardTitle.style.descriptionTextStyle, - wordWrapWidth: cardTitle._calculateTextWidth(), - textColor: cardTitle.style.descriptionTextStyle.textColor - }); + // textStyle has wordWrapWidth defined when the component generates rather than in the descriptionTextStyle + // thus the expect method cannot pass with "toEqual" hence the switch to "toMatchObject" + expect(cardTitle._Description.style.textStyle).toMatchObject( + cardTitle.style.descriptionTextStyle + ); + expect(cardTitle._Description.style.textStyle.wordWrapWidth).toEqual( + cardTitle._calculateTextWidth() + ); + expect(cardTitle._Description.style.textStyle.textColor).toEqual( + cardTitle.style.descriptionTextStyle.textColor + ); }); it('moves Description', () => { @@ -87,11 +93,12 @@ describe('CardTitle', () => { cardTitle.details = 'Details'; testRenderer.forceAllUpdates(); expect(cardTitle._Details.content).toEqual('Details'); - expect(cardTitle._Details.style.textStyle).toMatchObject({ - ...cardTitle.style.detailsTextStyle, - wordWrapWidth: cardTitle._calculateTextWidth(), - textColor: cardTitle.style.detailsTextStyle.textColor - }); + expect(cardTitle._Details.style.textStyle).toEqual( + expect.objectContaining(cardTitle.style.detailsTextStyle) + ); + expect(cardTitle._Details.style.textStyle.textColor).toEqual( + cardTitle.style.detailsTextStyle.textColor + ); }); it('moves Details', () => { diff --git a/packages/@lightningjs/ui-components/src/components/InlineContent/InlineContent.test.js b/packages/@lightningjs/ui-components/src/components/InlineContent/InlineContent.test.js index 4df6d3b86..a75d63fa4 100644 --- a/packages/@lightningjs/ui-components/src/components/InlineContent/InlineContent.test.js +++ b/packages/@lightningjs/ui-components/src/components/InlineContent/InlineContent.test.js @@ -112,7 +112,7 @@ describe('InlineContent', () => { inlineContent.content = 'This should be in testTheme font.'; await inlineContent.__updateSpyPromise; expect(context.theme.name).toBe('Test'); - expect(inlineContent.style.textStyle).toMatchObject({ + expect(inlineContent.style.textStyle).toEqual({ ...context.theme.typography.body1, verticalAlign: 'bottom' }); diff --git a/packages/@lightningjs/ui-components/src/components/Input/Input.js b/packages/@lightningjs/ui-components/src/components/Input/Input.js index d39a4e670..3753c1e6f 100644 --- a/packages/@lightningjs/ui-components/src/components/Input/Input.js +++ b/packages/@lightningjs/ui-components/src/components/Input/Input.js @@ -213,7 +213,7 @@ export default class Input extends Button { : this.cursorBlink.stop(); } this._Cursor.smooth = { - color: this.style.cursorStyle.color + color: this.style.cursorStyle.textColor }; } diff --git a/packages/@lightningjs/ui-components/src/components/Input/Input.styles.js b/packages/@lightningjs/ui-components/src/components/Input/Input.styles.js index 43c3b5612..2149df9bd 100644 --- a/packages/@lightningjs/ui-components/src/components/Input/Input.styles.js +++ b/packages/@lightningjs/ui-components/src/components/Input/Input.styles.js @@ -20,7 +20,7 @@ import { getWidthByUpCount } from '../../utils'; export const base = theme => ({ cursorStyle: { - color: theme.color.textNeutral, + textColor: theme.color.textNeutral, blink: true, width: theme.spacer.xs, height: theme.spacer.xxl @@ -46,7 +46,7 @@ export const mode = theme => ({ helpTextStyle: { textColor: theme.color.textNeutralDisabled } }, focused: { - cursorStyle: { color: theme.color.textInverse }, + cursorStyle: { textColor: theme.color.textInverse }, eyebrowTextStyle: { textColor: theme.color.textNeutral }, helpTextStyle: { textColor: theme.color.textNeutralSecondary } } diff --git a/packages/@lightningjs/ui-components/src/components/Input/__snapshots__/Input.test.js.snap b/packages/@lightningjs/ui-components/src/components/Input/__snapshots__/Input.test.js.snap index ece7dc898..9312aab12 100644 --- a/packages/@lightningjs/ui-components/src/components/Input/__snapshots__/Input.test.js.snap +++ b/packages/@lightningjs/ui-components/src/components/Input/__snapshots__/Input.test.js.snap @@ -53,7 +53,7 @@ exports[`Input renders 1`] = ` "attached": true, "boundsMargin": null, "clipping": false, - "color": "fff8f7fa", + "color": 4294967295, "enabled": true, "flex": false, "flexItem": false, diff --git a/packages/@lightningjs/ui-components/src/components/Keyboard/__snapshots__/Keyboard.test.js.snap b/packages/@lightningjs/ui-components/src/components/Keyboard/__snapshots__/Keyboard.test.js.snap index 37318cba3..5a1785fed 100644 --- a/packages/@lightningjs/ui-components/src/components/Keyboard/__snapshots__/Keyboard.test.js.snap +++ b/packages/@lightningjs/ui-components/src/components/Keyboard/__snapshots__/Keyboard.test.js.snap @@ -11692,7 +11692,7 @@ exports[`KeyboardInput renders 1`] = ` "attached": true, "boundsMargin": null, "clipping": false, - "color": "ff181819", + "color": 4294967295, "enabled": true, "flex": false, "flexItem": false, diff --git a/packages/@lightningjs/ui-components/src/components/MetadataCardContent/__snapshots__/MetadataCardContent.test.js.snap b/packages/@lightningjs/ui-components/src/components/MetadataCardContent/__snapshots__/MetadataCardContent.test.js.snap index 347c37ba0..f13db5fd1 100644 --- a/packages/@lightningjs/ui-components/src/components/MetadataCardContent/__snapshots__/MetadataCardContent.test.js.snap +++ b/packages/@lightningjs/ui-components/src/components/MetadataCardContent/__snapshots__/MetadataCardContent.test.js.snap @@ -294,6 +294,7 @@ exports[`MetadataCardContent renders 1`] = ` "precision": 0.6666666666666666, "text": "+7", "textBaseline": "bottom", + "textColor": 4294506490, "type": "TextTexture", "verticalAlign": "middle", }, diff --git a/packages/@lightningjs/ui-components/src/components/Provider/__snapshots__/Provider.test.js.snap b/packages/@lightningjs/ui-components/src/components/Provider/__snapshots__/Provider.test.js.snap index f08bf930f..0ecd2adbe 100644 --- a/packages/@lightningjs/ui-components/src/components/Provider/__snapshots__/Provider.test.js.snap +++ b/packages/@lightningjs/ui-components/src/components/Provider/__snapshots__/Provider.test.js.snap @@ -225,6 +225,7 @@ exports[`Provider renders 1`] = ` "precision": 0.6666666666666666, "text": "+1", "textBaseline": "bottom", + "textColor": 4294506490, "type": "TextTexture", "verticalAlign": "middle", }, diff --git a/packages/@lightningjs/ui-components/src/components/ScrollWrapper/__snapshots__/ScrollWrapper.test.js.snap b/packages/@lightningjs/ui-components/src/components/ScrollWrapper/__snapshots__/ScrollWrapper.test.js.snap index 67701d8d3..ae9d7e06a 100644 --- a/packages/@lightningjs/ui-components/src/components/ScrollWrapper/__snapshots__/ScrollWrapper.test.js.snap +++ b/packages/@lightningjs/ui-components/src/components/ScrollWrapper/__snapshots__/ScrollWrapper.test.js.snap @@ -494,6 +494,7 @@ exports[`ScrollWrapper renders 1`] = ` "precision": 0.6666666666666666, "text": "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu aliquam libero. Sed ipsum ligula, egestas et sollicitudin eget, pulvinar et neque. Curabitur commodo nisi sit amet ligula ultrices, a sodales ante consequat. Ut ullamcorper odio et erat sagittis volutpat. Cras consequat dolor in nisi sagittis, quis volutpat mi tempor. Praesent condimentum quis purus eget sodales. Praesent tempus suscipit felis, quis gravida massa tempor ut.", "textBaseline": "bottom", + "textColor": 4294506490, "type": "TextTexture", "verticalAlign": "middle", "wordWrapWidth": 48, @@ -662,6 +663,7 @@ exports[`ScrollWrapper renders a content array 1`] = ` "precision": 0.6666666666666666, "text": "Heading!", "textBaseline": "bottom", + "textColor": 4294506490, "type": "TextTexture", "verticalAlign": "middle", "wordWrapWidth": 48, @@ -749,6 +751,7 @@ exports[`ScrollWrapper renders a content array 1`] = ` "precision": 0.6666666666666666, "text": "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu aliquam libero. Sed ipsum ligula, egestas et sollicitudin eget, pulvinar et neque. Curabitur commodo nisi sit amet ligula ultrices, a sodales ante consequat. Ut ullamcorper odio et erat sagittis volutpat. Cras consequat dolor in nisi sagittis, quis volutpat mi tempor. Praesent condimentum quis purus eget sodales. Praesent tempus suscipit felis, quis gravida massa tempor ut.", "textBaseline": "bottom", + "textColor": 4294506490, "type": "TextTexture", "verticalAlign": "middle", "wordWrapWidth": 48, @@ -1312,6 +1315,7 @@ exports[`ScrollWrapper renders a content string 1`] = ` "precision": 0.6666666666666666, "text": "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu aliquam libero. Sed ipsum ligula, egestas et sollicitudin eget, pulvinar et neque. Curabitur commodo nisi sit amet ligula ultrices, a sodales ante consequat. Ut ullamcorper odio et erat sagittis volutpat. Cras consequat dolor in nisi sagittis, quis volutpat mi tempor. Praesent condimentum quis purus eget sodales. Praesent tempus suscipit felis, quis gravida massa tempor ut.", "textBaseline": "bottom", + "textColor": 4294506490, "type": "TextTexture", "verticalAlign": "middle", "wordWrapWidth": 48, diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js index 5140865c2..75cc13aa1 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js @@ -16,7 +16,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import lng from '@lightningjs/core'; import { clone, getValFromObjPath, getHexColor } from '../../utils'; import log from '../../globals/context/logger'; @@ -529,7 +528,7 @@ export const generateComponentStyleSource = ({ }, {}); const final = formatStyleObj( - removeEmptyObjects(themeParser({ theme }, solution)) || {}, + removeEmptyObjects(colorParser({ theme }, solution)) || {}, alias ); @@ -539,57 +538,12 @@ export const generateComponentStyleSource = ({ }; /** - * - * Default properties directly from @lightningjs/core to ensure correct fallback values - * - */ -export const lightningTextDefaults = Object.entries( - Object.getOwnPropertyDescriptors(lng.textures.TextTexture.prototype) -).reduce((acc, [prop]) => { - const value = lng.textures.TextTexture.prototype[prop]; - if (prop.startsWith('_') || ['undefined', 'function'].includes(typeof value)) - return acc; - return { - [prop]: value, - ...acc - }; -}, {}); - -// exclude properties are not unique to the Text Texture as they would not -// be reliable indicators that an object is a text style object -const textStyleProps = Object.keys(lightningTextDefaults).filter( - val => - ['paddingRight', 'paddingLeft', 'offsetY', 'h', 'w'].indexOf(val) === -1 -); - -/** - * Return whether or not an object contains properties of the Lightning Text texture. - * @param {object} obj an object to check i - * @returns {boolean} Returns true if the object contains properties of the Lightning Text texture. - */ -export const isTextStyleObject = obj => { - if (typeof obj !== 'object' || obj === null) { - return false; - } - // check if any of the fields in textStyleProps exist in the object - for (let i = 0; i < textStyleProps.length; i++) { - const field = textStyleProps[i]; - if (obj.hasOwnProperty(field)) { - return true; - } - } - return false; -}; - -/** - * Parse and process a style object to: - * - replace theme strings and process color arrays. - * - generate new text style objects to prevent merging with previous objects if the theme changes + * Parse and process a style object to replace theme strings and process color arrays. * @param {object} targetObject - In most cases, this will be a theme object. * @param {object} styleObj - The input style object to be processed. - * @returns {object} The processed style object with theme strings replaced and color arrays processed and new text style objects generated. + * @returns {object} The processed style object with theme strings replaced and color arrays processed. */ -export const themeParser = (targetObject, styleObj) => { +export const colorParser = (targetObject, styleObj) => { // Check if targetObject is an object if (typeof targetObject !== 'object' || targetObject === null) { throw new TypeError('targetObject must be an object.'); @@ -610,11 +564,6 @@ export const themeParser = (targetObject, styleObj) => { // Process value as a color ['#663399', 1] return getHexColor(value[0], value[1]); } - // Ensure text styles contain all default values from Text texture. - // This prevents properties that exist on a previous theme persisting on the current theme when switching themes by creating a new object each time. - if (typeof value === 'object' && isTextStyleObject(value)) { - return { ...lightningTextDefaults, ...value }; - } return value; }); diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js index 60b2c890e..e088bc4b3 100644 --- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js +++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js @@ -20,8 +20,7 @@ import { getStyleChain, formatStyleObj, replaceAliasValues, - themeParser, - lightningTextDefaults + colorParser } from './utils'; import log from '../../globals/context/logger'; @@ -36,7 +35,7 @@ jest.mock('./utils', () => ({ getValFromObjPath: jest.fn(), removeEmptyObjects: jest.fn(), getHexColor: jest.fn(), - themeParser: jest.fn() + colorParser: jest.fn() })); jest.mock('../../globals/context/logger', () => ({ @@ -922,7 +921,7 @@ describe('generateComponentStyleSource', () => { }); }); -describe('themeParser', () => { +describe('colorParser', () => { it('should parse style object with theme strings and color arrays', () => { const targetObject = { theme: { @@ -937,7 +936,7 @@ describe('themeParser', () => { borderColor: ['#663399', 1] }; - const result = themeParser(targetObject, styleObj); + const result = colorParser(targetObject, styleObj); const expectedOutput = { backgroundColor: '10', @@ -954,7 +953,7 @@ describe('themeParser', () => { const styleObj = {}; - const result = themeParser(targetObject, styleObj); + const result = colorParser(targetObject, styleObj); expect(result).toEqual({}); }); @@ -966,7 +965,7 @@ describe('themeParser', () => { backgroundColor: 'theme.radius.md' }; - expect(() => themeParser(targetObject, styleObj)).toThrow(TypeError); + expect(() => colorParser(targetObject, styleObj)).toThrow(TypeError); }); it('should throw a TypeError if styleObj is not an object', () => { @@ -976,28 +975,7 @@ describe('themeParser', () => { const styleObj = 'not an object'; // Passing a string instead of an object - expect(() => themeParser(targetObject, styleObj)).toThrow(TypeError); - }); - - it('should generate new text style objects', () => { - const targetObject = { - theme: {} - }; - - const styleObj = { - textStyle: { - fontSize: 12 - } - }; - - const result = themeParser(targetObject, styleObj); - - expect(result).toEqual({ - textStyle: { - ...lightningTextDefaults, - fontSize: 12 - } - }); + expect(() => colorParser(targetObject, styleObj)).toThrow(TypeError); }); });