feat: add form layout CSS properties, deprecate form item ones (#8691)

vaadin · Feb 19, 2025 · 1dc31c7 · 1dc31c7
1 parent ba77614
commit 1dc31c7
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 64 deletions.
diff --git a/packages/form-layout/src/vaadin-form-item-mixin.js b/packages/form-layout/src/vaadin-form-item-mixin.js
@@ -6,7 +6,9 @@
 import { addValueToAttribute, removeValueFromAttribute } from '@vaadin/component-base/src/dom-utils.js';
 import { generateUniqueId } from '@vaadin/component-base/src/unique-id-utils.js';
 
-let itemRowSpacingDeprecationNotified = false;
+let spacingDeprecationNotified = false;
+let labelWidthDeprecationNotified = false;
+let labelSpacingDeprecationNotified = false;
 
 /**
  * @polymerMixin
@@ -44,16 +46,34 @@ export const FormItemMixin = (superClass) =>
       this.__fieldNode = null;
     }
 
-    connectedCallback() {
-      super.connectedCallback();
-      if (!itemRowSpacingDeprecationNotified) {
-        const spacing = getComputedStyle(this).getPropertyValue('--vaadin-form-item-row-spacing');
-        if (spacing !== '' && parseInt(spacing) !== 0) {
-          console.warn(
-            '`--vaadin-form-item-row-spacing` is deprecated since 24.7. Use `--vaadin-form-layout-row-spacing` on <vaadin-form-layout> instead.',
-          );
-          itemRowSpacingDeprecationNotified = true;
-        }
+    /** @protected */
+    ready() {
+      super.ready();
+
+      const computedStyle = getComputedStyle(this);
+      const spacing = computedStyle.getPropertyValue('--vaadin-form-item-row-spacing');
+      const labelWidth = computedStyle.getPropertyValue('--vaadin-form-item-label-width');
+      const labelSpacing = computedStyle.getPropertyValue('--vaadin-form-item-label-spacing');
+
+      if (!spacingDeprecationNotified && spacing !== '' && parseInt(spacing) !== 0) {
+        console.warn(
+          '`--vaadin-form-item-row-spacing` is deprecated since 24.7. Use `--vaadin-form-layout-row-spacing` on <vaadin-form-layout> instead.',
+        );
+        spacingDeprecationNotified = true;
+      }
+
+      if (!labelWidthDeprecationNotified && labelWidth !== '' && parseInt(labelWidth) !== 0) {
+        console.warn(
+          '`--vaadin-form-item-label-width` is deprecated since 24.7. Use `--vaadin-form-layout-label-width` on <vaadin-form-layout> instead.',
+        );
+        labelWidthDeprecationNotified = true;
+      }
+
+      if (!labelSpacingDeprecationNotified && labelSpacing !== '' && parseInt(labelSpacing) !== 0) {
+        console.warn(
+          '`--vaadin-form-item-label-spacing` is deprecated since 24.7. Use `--vaadin-form-layout-label-spacing` on <vaadin-form-layout> instead.',
+        );
+        labelSpacingDeprecationNotified = true;
       }
     }
 

diff --git a/packages/form-layout/src/vaadin-form-item.d.ts b/packages/form-layout/src/vaadin-form-item.d.ts
@@ -87,8 +87,8 @@ import { FormItemMixin } from './vaadin-form-item-mixin.js';
  *
  * Custom CSS property | Description | Default
  * ---|---|---
- * `--vaadin-form-item-label-width` | Width of the label column when the labels are aside | `8em`
- * `--vaadin-form-item-label-spacing` | Spacing between the label column and the input column when the labels are aside | `1em`
+ * `--vaadin-form-item-label-width` | (DEPRECATED: Use `--vaadin-form-layout-label-width` on `<vaadin-form-layout>` instead) Width of the label column when the labels are aside | `8em`
+ * `--vaadin-form-item-label-spacing` | (DEPRECATED: Use `--vaadin-form-layout-label-spacing` on `<vaadin-form-layout>` instead) Spacing between the label column and the input column when the labels are aside | `1em`
  * `--vaadin-form-item-row-spacing` | (DEPRECATED: Use `--vaadin-form-layout-row-spacing` on `<vaadin-form-layout>` instead) Height of the spacing between the form item elements | `1em`
  *
  * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation.

diff --git a/packages/form-layout/src/vaadin-form-item.js b/packages/form-layout/src/vaadin-form-item.js
@@ -92,8 +92,8 @@ registerStyles('vaadin-form-item', formItemStyles, { moduleId: 'vaadin-form-item
  *
  * Custom CSS property | Description | Default
  * ---|---|---
- * `--vaadin-form-item-label-width` | Width of the label column when the labels are aside | `8em`
- * `--vaadin-form-item-label-spacing` | Spacing between the label column and the input column when the labels are aside | `1em`
+ * `--vaadin-form-item-label-width` | (DEPRECATED: Use `--vaadin-form-layout-label-width` on `<vaadin-form-layout>` instead) Width of the label column when the labels are aside | `8em`
+ * `--vaadin-form-item-label-spacing` | (DEPRECATED: Use `--vaadin-form-layout-label-spacing` on `<vaadin-form-layout>` instead) Spacing between the label column and the input column when the labels are aside | `1em`
  * `--vaadin-form-item-row-spacing` | (DEPRECATED: Use `--vaadin-form-layout-row-spacing` on `<vaadin-form-layout>` instead) Height of the spacing between the form item elements | `1em`
  *
  * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation.

diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js
@@ -7,13 +7,14 @@ import { css } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 
 export const formLayoutStyles = css`
   :host {
+    /* Default values */
+    --vaadin-form-layout-row-spacing: 1em;
+    --vaadin-form-layout-column-spacing: 2em;
+    --vaadin-form-layout-label-width: 8em;
+    --vaadin-form-layout-label-spacing: 1em;
+
     display: block;
     max-width: 100%;
-    /* CSS API for host */
-    --vaadin-form-item-label-width: 8em;
-    --vaadin-form-item-label-spacing: 1em;
-    --vaadin-form-layout-row-spacing: 1em;
-    --vaadin-form-layout-column-spacing: 2em; /* (default) */
     align-self: stretch;
   }
 
@@ -62,7 +63,7 @@ export const formItemStyles = css`
   }
 
   #label {
-    width: var(--vaadin-form-item-label-width, 8em);
+    width: var(--vaadin-form-item-label-width, var(--vaadin-form-layout-label-width, 8em));
     flex: 0 0 auto;
   }
 
@@ -71,7 +72,7 @@ export const formItemStyles = css`
   }
 
   #spacing {
-    width: var(--vaadin-form-item-label-spacing, 1em);
+    width: var(--vaadin-form-item-label-spacing, var(--vaadin-form-layout-label-spacing, 1em));
     flex: 0 0 auto;
   }
 

diff --git a/packages/form-layout/src/vaadin-form-layout.js b/packages/form-layout/src/vaadin-form-layout.js
@@ -101,7 +101,9 @@ registerStyles('vaadin-form-layout', formLayoutStyles, { moduleId: 'vaadin-form-
  * Custom CSS property | Description | Default
  * ---|---|---
  * `--vaadin-form-layout-column-spacing` | Length of the spacing between columns | `2em`
- * `--vaadin-form-layout-row-spacing` | Length of the spacing between rows | 1em`
+ * `--vaadin-form-layout-row-spacing` | Length of the spacing between rows | `1em`
+ * `--vaadin-form-layout-label-width` | Width of the label when labels are displayed aside | `8em`
+ * `--vaadin-form-layout-label-spacing` | Length of the spacing between the label and the input when labels are displayed aside | `1em`
  *
  * @customElement
  * @extends HTMLElement

diff --git a/packages/form-layout/test/form-layout.test.js b/packages/form-layout/test/form-layout.test.js
@@ -2,9 +2,8 @@ import { expect } from '@vaadin/chai-plugins';
 import { aTimeout, fixtureSync, nextRender, nextResize } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import '@vaadin/text-field/vaadin-text-field.js';
-import '../vaadin-form-layout.js';
-import '../vaadin-form-item.js';
-import '@polymer/polymer/lib/elements/dom-repeat.js';
+import '../src/vaadin-form-layout.js';
+import '../src/vaadin-form-item.js';
 
 function estimateEffectiveColumnCount(layout) {
   const offsets = [...layout.children]
@@ -57,68 +56,150 @@ describe('form layout', () => {
 
     beforeEach(() => {
       layout = fixtureSync(`
-        <vaadin-form-layout>
-          <vaadin-text-field></vaadin-text-field>
+        <vaadin-form-layout style="font-size: 16px;">
+          <input />
           <vaadin-form-item></vaadin-form-item>
+          <input />
         </vaadin-form-layout>
       `);
+      layout.responsiveSteps = [{ columns: 3 }];
       item = layout.querySelector('vaadin-form-item');
     });
 
-    it('should have default label-width', () => {
-      expect(getComputedStyle(item).getPropertyValue('--vaadin-form-item-label-width').trim()).to.equal('8em');
-      const labelFontSize = parseFloat(getComputedStyle(item.$.label).fontSize);
-      expect(parseFloat(getComputedStyle(item.$.label).width)).to.be.closeTo(8 * labelFontSize, 0.5);
+    it('should apply default --vaadin-form-layout-label-width', () => {
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-label-width')).to.equal('8em');
+      expect(getComputedStyle(item.$.label).width).to.equal('128px');
     });
 
-    it('should apply label-width', () => {
-      item.style.setProperty('--vaadin-form-item-label-width', '100px');
-      expect(getComputedStyle(item.$.label).width).to.equal('100px');
+    it('should apply default --vaadin-form-layout-label-spacing', () => {
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-label-spacing')).to.equal('1em');
+      expect(getComputedStyle(item.$.spacing).width).to.equal('16px');
+    });
+
+    it('should apply default --vaadin-form-layout-row-spacing', () => {
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-row-spacing')).to.equal('1em');
+      expect(getComputedStyle(item).marginTop).to.equal('8px');
+      expect(getComputedStyle(item).marginBottom).to.equal('8px');
     });
 
-    it('should not apply label-width when label-position="top" attribute is set', () => {
+    it('should apply default --vaadin-form-layout-column-spacing', () => {
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-column-spacing')).to.equal('2em');
+      expect(getComputedStyle(item).marginLeft).to.equal('16px');
+      expect(getComputedStyle(item).marginRight).to.equal('16px');
+    });
+
+    it('should ignore --vaadin-form-layout-label-width when label-position is top', () => {
       item.setAttribute('label-position', 'top');
-      item.style.setProperty('--vaadin-form-item-label-width', '100px');
-      expect(getComputedStyle(item.$.label).width).to.not.equal('100px');
+      expect(getComputedStyle(item.$.label).width).to.not.equal('128px');
     });
 
-    it('should have default label-spacing', () => {
-      expect(getComputedStyle(item).getPropertyValue('--vaadin-form-item-label-spacing').trim()).to.equal('1em');
-      expect(getComputedStyle(item.$.spacing).width).to.equal('16px'); // 1em in px
+    it('should apply custom --vaadin-form-layout-label-width when set', () => {
+      layout.style.setProperty('--vaadin-form-layout-label-width', '100px');
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-label-width')).to.equal('100px');
+      expect(getComputedStyle(item.$.label).width).to.equal('100px');
     });
 
-    it('should apply label-spacing', () => {
-      item.style.setProperty('--vaadin-form-item-label-spacing', '8px');
+    it('should apply custom --vaadin-form-layout-label-spacing when set', () => {
+      layout.style.setProperty('--vaadin-form-layout-label-spacing', '8px');
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-label-spacing')).to.equal('8px');
       expect(getComputedStyle(item.$.spacing).width).to.equal('8px');
     });
 
-    it('should not have default row-spacing', () => {
-      expect(getComputedStyle(item).getPropertyValue('--vaadin-form-layout-row-spacing').trim()).to.equal('0');
-      expect(parseFloat(getComputedStyle(item).marginTop)).to.equal(0);
-      expect(parseFloat(getComputedStyle(item).marginBottom)).to.equal(0);
+    it('should apply custom --vaadin-form-layout-row-spacing when set', () => {
+      layout.style.setProperty('--vaadin-form-layout-row-spacing', '8px');
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-row-spacing')).to.equal('8px');
+      expect(getComputedStyle(item).marginTop).to.equal('4px');
+      expect(getComputedStyle(item).marginBottom).to.equal('4px');
+    });
+
+    it('should apply custom --vaadin-form-layout-column-spacing when set', () => {
+      layout.style.setProperty('--vaadin-form-layout-column-spacing', '8px');
+      expect(getComputedStyle(layout).getPropertyValue('--vaadin-form-layout-column-spacing')).to.equal('8px');
+      expect(getComputedStyle(item).marginLeft).to.equal('4px');
+      expect(getComputedStyle(item).marginRight).to.equal('4px');
     });
+  });
+
+  describe('deprecated CSS properties', () => {
+    let layout, item;
+
+    beforeEach(() => {
+      sinon.stub(console, 'warn');
 
-    it('should apply form layout row spacing', () => {
-      const spacing = 8;
-      item.style.setProperty('--vaadin-form-layout-row-spacing', `${spacing}px`);
-      expect(parseFloat(getComputedStyle(item).marginTop)).to.equal(spacing / 2);
-      expect(parseFloat(getComputedStyle(item).marginBottom)).to.equal(spacing / 2);
+      layout = fixtureSync(`
+        <vaadin-form-layout style="font-size: 16px;">
+          <input />
+          <vaadin-form-item></vaadin-form-item>
+          <input />
+        </vaadin-form-layout>
+      `);
+      layout.responsiveSteps = [{ columns: 3 }];
+      item = layout.querySelector('vaadin-form-item');
     });
 
-    it('should apply form item row spacing', () => {
-      const spacing = 8;
-      item.style.setProperty('--vaadin-form-item-row-spacing', `${spacing}px`);
-      expect(parseFloat(getComputedStyle(item).marginTop)).to.equal(spacing / 2);
-      expect(parseFloat(getComputedStyle(item).marginBottom)).to.equal(spacing / 2);
+    afterEach(() => {
+      console.warn.restore();
     });
 
-    it('should apply default column-spacing', async () => {
-      // Override to not depend on the theme changes
-      layout.style.setProperty('--lumo-space-l', '2rem');
-      await nextResize(layout);
-      expect(getParsedWidth(layout.firstElementChild).spacing).to.equal('1rem');
-      expect(getComputedStyle(layout.firstElementChild).getPropertyValue('margin-left')).to.equal('0px'); // Zero because it's first
-      expect(getComputedStyle(layout.firstElementChild).getPropertyValue('margin-right')).to.equal('16px'); // 0.5 * 2rem in px
+    it('should not have deprecated --vaadin-form-item-label-width by default', () => {
+      expect(getComputedStyle(item).getPropertyValue('--vaadin-form-item-label-width')).to.be.empty;
+    });
+
+    it('should not have deprecated --vaadin-form-item-label-spacing by default', () => {
+      expect(getComputedStyle(item).getPropertyValue('--vaadin-form-item-label-spacing')).to.be.empty;
+    });
+
+    it('should not have deprecated --vaadin-form-item-row-spacing by default', () => {
+      expect(getComputedStyle(item).getPropertyValue('--vaadin-form-item-row-spacing')).to.be.empty;
+    });
+
+    it('should warn once when adding form items with deprecated --vaadin-form-item-label-width', () => {
+      for (let i = 0; i < 2; i++) {
+        const item = document.createElement('vaadin-form-item');
+        item.style.setProperty('--vaadin-form-item-label-width', '100px');
+        layout.appendChild(item);
+      }
+
+      expect(console.warn).to.be.calledOnce;
+      expect(console.warn.args[0][0]).to.contain('`--vaadin-form-item-label-width` is deprecated');
+    });
+
+    it('should warn once when adding form items with deprecated --vaadin-form-item-label-spacing', () => {
+      for (let i = 0; i < 2; i++) {
+        const item = document.createElement('vaadin-form-item');
+        item.style.setProperty('--vaadin-form-item-label-spacing', '8px');
+        layout.appendChild(item);
+      }
+
+      expect(console.warn).to.be.calledOnce;
+      expect(console.warn.args[0][0]).to.contain('`--vaadin-form-item-label-spacing` is deprecated');
+    });
+
+    it('should warn when adding form items with deprecated --vaadin-form-item-row-spacing', () => {
+      for (let i = 0; i < 2; i++) {
+        const item = document.createElement('vaadin-form-item');
+        item.style.setProperty('--vaadin-form-item-row-spacing', '8px');
+        layout.appendChild(item);
+      }
+
+      expect(console.warn).to.be.calledOnce;
+      expect(console.warn.args[0][0]).to.contain('`--vaadin-form-item-row-spacing` is deprecated');
+    });
+
+    it('should apply deprecated --vaadin-form-item-label-spacing when set', () => {
+      item.style.setProperty('--vaadin-form-item-label-spacing', '8px');
+      expect(getComputedStyle(item.$.spacing).width).to.equal('8px');
+    });
+
+    it('should apply deprecated --vaadin-form-item-label-width when set', () => {
+      item.style.setProperty('--vaadin-form-item-label-width', '100px');
+      expect(getComputedStyle(item.$.label).width).to.equal('100px');
+    });
+
+    it('should apply deprecated --vaadin-form-item-row-spacing when set', () => {
+      item.style.setProperty('--vaadin-form-item-row-spacing', '8px');
+      expect(getComputedStyle(item).marginTop).to.equal('4px');
+      expect(getComputedStyle(item).marginBottom).to.equal('4px');
     });
   });