refactor!: use CSS gap for row and column spacing

packages/form-layout/src/vaadin-form-item-mixin.js

@@ -50,7 +50,13 @@ export const FormItemMixin = (superClass) =>
         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.',
+            `
+              --vaadin-form-item-row-spacing is deprecated since 24.7. Instead you should now:
+              1. Use --vaadin-form-layout-row-spacing on the <vaadin-form-layout> component to control the gap between rows
+              2. Use standard CSS margin on <vaadin-form-layout> to control the spacing around the form layout itself
+            `
+              .trim()
+              .replace(/\s{2,}/gu, '\n'),
           );
           itemRowSpacingDeprecationNotified = true;
         }

packages/form-layout/src/vaadin-form-layout-mixin.js

@@ -282,10 +282,6 @@ export const FormLayoutMixin = (superClass) =>
       const style = getComputedStyle(this);
       const columnSpacing = style.getPropertyValue('--vaadin-form-layout-column-spacing');
 
-      const direction = style.direction;
-      const marginStartProp = `margin-${direction === 'ltr' ? 'left' : 'right'}`;
-      const marginEndProp = `margin-${direction === 'ltr' ? 'right' : 'left'}`;
-
       const containerWidth = this.offsetWidth;
 
       let col = 0;
@@ -313,27 +309,14 @@ export const FormLayoutMixin = (superClass) =>
             col = 0;
           }
 
-          // At the start edge
-          if (col === 0) {
-            child.style.setProperty(marginStartProp, '0px');
-          } else {
-            child.style.removeProperty(marginStartProp);
-          }
-
           const nextIndex = index + 1;
           const nextLineBreak = nextIndex < children.length && children[nextIndex].localName === 'br';
 
-          // At the end edge
-          if (col + colspan === this._columnCount) {
-            child.style.setProperty(marginEndProp, '0px');
-          } else if (nextLineBreak) {
+          if (nextLineBreak) {
             const colspanRatio = (this._columnCount - col - colspan) / this._columnCount;
-            child.style.setProperty(
-              marginEndProp,
-              `calc(${colspanRatio * containerWidth}px + ${colspanRatio} * ${columnSpacing})`,
-            );
+            child.style.marginInlineEnd = `calc(${colspanRatio * containerWidth}px + ${colspanRatio} * ${columnSpacing})`;
           } else {
-            child.style.removeProperty(marginEndProp);
+            child.style.marginInlineEnd = '';
           }
 
           // Move the column counter

packages/form-layout/src/vaadin-form-layout-styles.js

@@ -12,7 +12,7 @@ export const formLayoutStyles = css`
     /* 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-row-spacing: 0;
     --vaadin-form-layout-column-spacing: 2em; /* (default) */
     align-self: stretch;
   }
@@ -23,20 +23,15 @@ export const formLayoutStyles = css`
 
   #layout {
     display: flex;
-
     align-items: baseline; /* default \`stretch\` is not appropriate */
-
     flex-wrap: wrap; /* the items should wrap */
+    gap: var(--vaadin-form-layout-row-spacing) var(--vaadin-form-layout-column-spacing);
   }
 
   #layout ::slotted(*) {
     /* Items should neither grow nor shrink. */
     flex-grow: 0;
     flex-shrink: 0;
-
-    /* Margins make spacing between the columns */
-    margin-left: calc(0.5 * var(--vaadin-form-layout-column-spacing));
-    margin-right: calc(0.5 * var(--vaadin-form-layout-column-spacing));
   }
 
   #layout ::slotted(br) {
@@ -49,7 +44,15 @@ export const formItemStyles = css`
     display: inline-flex;
     flex-direction: row;
     align-items: baseline;
-    margin: calc(0.5 * var(--vaadin-form-item-row-spacing, var(--vaadin-form-layout-row-spacing, 1em))) 0;
+
+    /*
+      WARNING: --vaadin-form-item-row-spacing is deprecated since 24.7. Instead you should now:
+      1. Use --vaadin-form-layout-row-spacing on the <vaadin-form-layout> component to control the gap between rows.
+      2. Use standard CSS margin on <vaadin-form-layout> to control the spacing around the form layout itself.
+    */
+    --_has-layout-row-spacing: clamp(0, var(--vaadin-form-layout-row-spacing), 1);
+    --_item-row-spacing: calc((1 - var(--_has-layout-row-spacing)) * var(--vaadin-form-item-row-spacing, 1em));
+    margin: calc(var(--_item-row-spacing) / 2) 0;
   }
 
   :host([label-position='top']) {

packages/form-layout/test/form-layout.test.js

@@ -94,31 +94,34 @@ describe('form layout', () => {
 
     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);
+      expect(getComputedStyle(item).marginTop).to.equal('0px');
+      expect(getComputedStyle(item).marginBottom).to.equal('0px');
     });
 
     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.style.setProperty('--vaadin-form-layout-row-spacing', '8px');
+      expect(getComputedStyle(layout.$.layout).rowGap).to.equal('8px');
     });
 
     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);
+      expect(getComputedStyle(item).marginTop).to.equal(`${spacing / 2}px`);
+      expect(getComputedStyle(item).marginBottom).to.equal(`${spacing / 2}px`);
+    });
+
+    it('should not apply form item row spacing when form layout spacing is non-zero', () => {
+      layout.style.setProperty('--vaadin-form-layout-row-spacing', '8px');
+      item.style.setProperty('--vaadin-form-item-row-spacing', `8px`);
+      expect(getComputedStyle(item).marginTop).to.equal('0px');
+      expect(getComputedStyle(item).marginBottom).to.equal('0px');
     });
 
     it('should apply default column-spacing', async () => {
       // Override to not depend on the theme changes
-      layout.style.setProperty('--lumo-space-l', '2rem');
+      layout.style.setProperty('--lumo-space-l', '8px');
       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
+      expect(getComputedStyle(layout.$.layout).columnGap).to.equal('8px');
     });
   });
 
@@ -531,29 +534,24 @@ describe('form layout', () => {
 
       newFormItem.hidden = false;
       await nextRender(container);
-      const unhiddenItemWidth = newFormItem.getBoundingClientRect().width;
-      expect(unhiddenItemWidth).to.equal(itemWidth);
+      expect(newFormItem.getBoundingClientRect().width).to.equal(itemWidth);
     });
 
-    it('should update layout after a new item is added', async () => {
-      const newFormItem = document.createElement('vaadin-form-item');
-      newFormItem.innerHTML = '<label slot="label">Field</label><input />';
-      layout.insertBefore(newFormItem, items[0]);
-      await nextRender(container);
-      expect(getComputedStyle(newFormItem).marginLeft).to.be.equal('0px');
+    it('should update margin-inline-end after adding <br>', async () => {
+      const br = document.createElement('br');
+      layout.insertBefore(br, items[1]);
+      await nextRender(layout);
+      expect(getComputedStyle(items[0]).marginInlineEnd).to.not.equal('0px');
     });
 
-    it('should update layout after an item is removed', async () => {
-      const newFormItem = document.createElement('vaadin-form-item');
-      newFormItem.innerHTML = '<label slot="label">Field</label><input />';
-      layout.insertBefore(newFormItem, items[0]);
-      await nextRender(container);
+    it('should update margin-inline-end after removing <br>', async () => {
+      const br = document.createElement('br');
+      layout.insertBefore(br, items[1]);
+      await nextRender(layout);
 
-      expect(getComputedStyle(items[0]).marginLeft).to.not.be.equal('0px');
-
-      newFormItem.remove();
-      await nextRender(container);
-      expect(getComputedStyle(items[0]).marginLeft).to.be.equal('0px');
+      br.remove();
+      await nextRender(layout);
+      expect(getComputedStyle(items[0]).marginInlineEnd).to.equal('0px');
     });
 
     it('should not update layout when setting hidden to true', async () => {

packages/form-layout/theme/lumo/vaadin-form-layout-styles.js

@@ -6,7 +6,7 @@ registerStyles(
   css`
     :host {
       --vaadin-form-layout-column-spacing: var(--lumo-space-l);
-      --vaadin-form-layout-row-spacing: 0;
+      --vaadin-form-item-row-spacing: 0;
     }
   `,
   { moduleId: 'lumo-form-layout' },