feat(Button): update default style mappings and calculate height dyna…

…mically (#394) * fix(Button): alias titlePadding to contentSpacing * fix(Button): add paddingY and radius style properties * fix(Button): calculate height based on lineHeight and paddingY * test(Button): update snapshots and ButtonSmall and ControlSmall height tests
rdkcentral · Oct 30, 2023 · 77ccf44 · 77ccf44
1 parent 3c5e13a
commit 77ccf44
Show file tree

Hide file tree

Showing 19 changed files with 182 additions and 134 deletions.
diff --git a/packages/@lightningjs/ui-components/src/components/Button/Button.d.ts b/packages/@lightningjs/ui-components/src/components/Button/Button.d.ts
@@ -26,7 +26,10 @@ type ButtonStyle = SurfaceStyle & {
   minWidth: number;
   paddingX: number;
   paddingXNoTitle: number;
+  paddingY: number;
+  /** @deprecated */
   titlePadding: number;
+  contentSpacing: number;
   textStyle: TextBoxStyle;
   contentColor: Color;
 };

diff --git a/packages/@lightningjs/ui-components/src/components/Button/Button.js b/packages/@lightningjs/ui-components/src/components/Button/Button.js
@@ -39,6 +39,10 @@ export default class Button extends Surface {
     return ['fixed', 'justify', 'prefix', 'suffix', 'title'];
   }
 
+  static get aliasStyles() {
+    return [{ prev: 'titlePadding', curr: 'contentSpacing' }];
+  }
+
   static get tags() {
     return [
       ...super.tags,
@@ -256,6 +260,11 @@ export default class Button extends Surface {
     if (newWidth !== this.w) {
       this.w = newWidth;
     }
+
+    this.h =
+      this.style.height ||
+      this.style.textStyle.lineHeight + this.style.paddingY * 2;
+
     // TODO breaks row resizing if this is wrapped in the above conditional
     this.fireAncestors('$itemChanged');
   }
@@ -335,7 +344,7 @@ export default class Button extends Surface {
   }
 
   get _titleX() {
-    return this._hasPrefix ? this._prefixW + this.style.titlePadding : 0;
+    return this._hasPrefix ? this._prefixW + this.style.contentSpacing : 0;
   }
 
   get _hasSuffix() {
@@ -348,7 +357,7 @@ export default class Button extends Surface {
 
   get _suffixX() {
     if (this._hasTitle) {
-      return this._titleW + this._TextWrapper.x + this.style.titlePadding;
+      return this._titleW + this._TextWrapper.x + this.style.contentSpacing;
     } else if (this._hasPrefix) {
       return this._prefixW + this.style.itemSpacing;
     }
@@ -378,10 +387,10 @@ export default class Button extends Surface {
   get _totalTitlePaddingX() {
     let totalTitlePadding = 0;
     if (this._hasPrefix) {
-      totalTitlePadding += this.style.titlePadding;
+      totalTitlePadding += this.style.contentSpacing;
     }
     if (this._hasSuffix) {
-      totalTitlePadding += this.style.titlePadding;
+      totalTitlePadding += this.style.contentSpacing;
     }
     return totalTitlePadding;
   }

diff --git a/packages/@lightningjs/ui-components/src/components/Button/Button.mdx b/packages/@lightningjs/ui-components/src/components/Button/Button.mdx
@@ -82,4 +82,4 @@ If you'd like to have a fixed button with a width smaller than the `minWidth`, y
 | paddingX        | number                        | space between the button horizontal edge and the content                               |
 | paddingXNoTitle | number                        | space between the button horizontal edge and the content if no title is provided       |
 | textStyle       | string \| object              | text style to apply to the title                                                       |
-| titlePadding    | number                        | padding between title and prefix                                                       |
+| contentSpacing  | number                        | padding between title and prefix                                                       |
diff --git a/packages/@lightningjs/ui-components/src/components/Button/Button.styles.js b/packages/@lightningjs/ui-components/src/components/Button/Button.styles.js
@@ -19,13 +19,14 @@
 import { getWidthByColumnSpan } from '../../utils';
 
 export const base = theme => ({
-  height: theme.spacer.md * 10,
   width: 0,
   justify: 'center',
   minWidth: getWidthByColumnSpan(theme, 3),
   paddingX: theme.spacer.xxxl,
   paddingXNoTitle: theme.spacer.xl,
-  titlePadding: theme.spacer.md,
+  paddingY: theme.spacer.xl,
+  radius: theme.radius.sm,
+  contentSpacing: theme.spacer.md,
   itemSpacing: theme.spacer.md,
   textStyle: {
     ...theme.typography.button1,

diff --git a/packages/@lightningjs/ui-components/src/components/Button/ButtonSmall.styles.js b/packages/@lightningjs/ui-components/src/components/Button/ButtonSmall.styles.js
@@ -19,8 +19,9 @@
 import { getWidthByColumnSpan } from '../../utils';
 
 export const base = theme => ({
-  height: theme.spacer.md * 8,
   minWidth: getWidthByColumnSpan(theme, 1),
   paddingX: theme.spacer.xxl,
+  paddingXNoTitle: theme.spacer.lg,
+  paddingY: theme.spacer.lg,
   textStyle: theme.typography.button2
 });
diff --git a/packages/@lightningjs/ui-components/src/components/Button/ButtonSmall.test.js b/packages/@lightningjs/ui-components/src/components/Button/ButtonSmall.test.js
@@ -46,8 +46,16 @@ describe('ButtonSmall', () => {
     expect(tree).toMatchSnapshot();
   });
 
-  it('renders the correct height', () => {
-    expect(buttonSmall.h).toBe(buttonSmall.style.h);
+  it('renders the correct height from styles', () => {
+    expect(buttonSmall.h).toBe(
+      buttonSmall.style.textStyle.lineHeight + buttonSmall.style.paddingY * 2
+    );
+  });
+
+  it('renders the correct height if assigned a height', () => {
+    const height = 50;
+    buttonSmall.h = height;
+    expect(buttonSmall.h).toBe(height);
   });
 
   it('renders the correct width', () => {

diff --git a/packages/@lightningjs/ui-components/src/components/Control/Control.js b/packages/@lightningjs/ui-components/src/components/Control/Control.js
@@ -33,6 +33,10 @@ export default class Control extends ButtonSmall {
     return [...super.properties, 'icon', 'logo', 'shouldCollapse'];
   }
 
+  static get aliasStyles() {
+    return [{ prev: 'titlePadding', curr: 'contentSpacing' }];
+  }
+
   _update() {
     // ordering this way to make sure that this._Title is defined so the title visibility can be set properly when _updateCollapseStatus is called
     this._updatePrefixStyle();
@@ -91,7 +95,7 @@ export default class Control extends ButtonSmall {
               (this._paddingLeft +
                 this._paddingRight +
                 this._Prefix.w +
-                this.style.titlePadding);
+                this.style.contentSpacing);
 
             this._patchTitle(leftOverSpace, 1);
           }
@@ -114,7 +118,7 @@ export default class Control extends ButtonSmall {
               (this.w -
                 (this._paddingLeft +
                   this._Prefix.w +
-                  this.style.titlePadding +
+                  this.style.contentSpacing +
                   this._paddingRight)) /
               2;
             this._patchTitle(middle, 0.5);

diff --git a/packages/@lightningjs/ui-components/src/components/Control/Control.styles.js b/packages/@lightningjs/ui-components/src/components/Control/Control.styles.js
@@ -38,6 +38,6 @@ export const base = theme => {
     paddingXNoTitle: theme.spacer.md,
     prefixPadding: theme.spacer.md,
     radius,
-    titlePadding: theme.spacer.md
+    contentSpacing: theme.spacer.md
   };
 };
diff --git a/packages/@lightningjs/ui-components/src/components/Control/Control.test.js b/packages/@lightningjs/ui-components/src/components/Control/Control.test.js
@@ -265,7 +265,7 @@ describe('Control', () => {
             (control._paddingLeft +
               control._paddingRight +
               control._Prefix.w +
-              control.style.titlePadding)
+              control.style.contentSpacing)
         );
         expect(control._Title.mountX).toEqual(1);
       });
@@ -327,7 +327,7 @@ describe('Control', () => {
           (control.w -
             (control._paddingLeft +
               control._Prefix.w +
-              control.style.titlePadding) -
+              control.style.contentSpacing) -
             control._paddingRight) /
             2
         );

diff --git a/packages/@lightningjs/ui-components/src/components/Control/ControlSmall.test.js b/packages/@lightningjs/ui-components/src/components/Control/ControlSmall.test.js
@@ -58,8 +58,16 @@ describe('ControlSmall', () => {
     );
   });
 
-  it('renders the correct height', async () => {
-    expect(controlSmall.h).toBe(controlSmall.style.h);
+  it('renders the correct height from styles', () => {
+    expect(controlSmall.h).toBe(
+      controlSmall.style.textStyle.lineHeight + controlSmall.style.paddingY * 2
+    );
+  });
+
+  it('renders the correct height if assigned a height', () => {
+    const height = 50;
+    controlSmall.h = height;
+    expect(controlSmall.h).toBe(height);
   });
 
   it('renders the correct radius', async () => {

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
@@ -186,7 +186,7 @@ exports[`Input renders 1`] = `
       "visible": true,
       "w": 0,
       "x": 30,
-      "y": 50,
+      "y": 0,
       "zIndex": 2,
     },
   },