Revert "fix: update global typography props when new theme is set (#432…

…)" (#442) This reverts commit 28d3f95. Co-authored-by: Emily Rautenberg <[email protected]>
rdkcentral · Dec 20, 2023 · ca3d2d1 · ca3d2d1
1 parent 494d7e8
commit ca3d2d1
Show file tree

Hide file tree

Showing 13 changed files with 49 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...es/@lightningjs/ui-components/src/components/Keyboard/__snapshots__/Keyboard.test.js.snap b/...es/@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/...ponents/src/components/MetadataCardContent/__snapshots__/MetadataCardContent.test.js.snap b/...ponents/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/...es/@lightningjs/ui-components/src/components/Provider/__snapshots__/Provider.test.js.snap b/...es/@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/...ingjs/ui-components/src/components/ScrollWrapper/__snapshots__/ScrollWrapper.test.js.snap b/...ingjs/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
@@ -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
@@ -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);
   });
 });