diff --git a/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-edit.png b/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-edit.png index 7e6acce90c7..ee5b38373da 100644 Binary files a/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-edit.png and b/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-edit.png differ diff --git a/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-new.png b/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-new.png index 760fc663da4..dd75e5633b7 100644 Binary files a/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-new.png and b/packages/crud/test/visual/lumo/screenshots/crud/baseline/ltr-editor-position-default-new.png differ diff --git a/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-edit.png b/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-edit.png index 078e808e5b9..15cd2275d30 100644 Binary files a/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-edit.png and b/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-edit.png differ diff --git a/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-new.png b/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-new.png index 66952421d2b..d39794ffe68 100644 Binary files a/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-new.png and b/packages/crud/test/visual/lumo/screenshots/crud/baseline/rtl-editor-position-default-new.png differ diff --git a/packages/crud/test/visual/lumo/screenshots/crud/baseline/theme-no-border-edit.png b/packages/crud/test/visual/lumo/screenshots/crud/baseline/theme-no-border-edit.png index 92a0c76bd7c..edfbda885b4 100644 Binary files a/packages/crud/test/visual/lumo/screenshots/crud/baseline/theme-no-border-edit.png and b/packages/crud/test/visual/lumo/screenshots/crud/baseline/theme-no-border-edit.png differ diff --git a/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-edit.png b/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-edit.png index 1123dd02b6e..003eaec3a70 100644 Binary files a/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-edit.png and b/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-edit.png differ diff --git a/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-new.png b/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-new.png index ba6fb446d7f..2848ebe69ed 100644 Binary files a/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-new.png and b/packages/crud/test/visual/material/screenshots/crud/baseline/ltr-editor-position-default-new.png differ diff --git a/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-edit.png b/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-edit.png index 531173d88a0..d7a9dce2ed3 100644 Binary files a/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-edit.png and b/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-edit.png differ diff --git a/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-new.png b/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-new.png index e5a79e35b0a..4f9691f7d3e 100644 Binary files a/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-new.png and b/packages/crud/test/visual/material/screenshots/crud/baseline/rtl-editor-position-default-new.png differ diff --git a/packages/form-layout/src/vaadin-form-item-mixin.js b/packages/form-layout/src/vaadin-form-item-mixin.js index c462d79b71b..2b8bd36e744 100644 --- a/packages/form-layout/src/vaadin-form-item-mixin.js +++ b/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 instead.', + ` + --vaadin-form-item-row-spacing is deprecated since 24.7. Instead you should now: + 1. Use --vaadin-form-layout-row-spacing on the component to control the gap between rows + 2. Use standard CSS margin on to control the spacing around the form layout itself + ` + .trim() + .replace(/\s{2,}/gu, '\n'), ); itemRowSpacingDeprecationNotified = true; } diff --git a/packages/form-layout/src/vaadin-form-layout-mixin.js b/packages/form-layout/src/vaadin-form-layout-mixin.js index a556dfd3560..402750c4eb7 100644 --- a/packages/form-layout/src/vaadin-form-layout-mixin.js +++ b/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 diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index 1bbf41dee2d..8e16d5cc2d3 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/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 component to control the gap between rows. + 2. Use standard CSS margin on 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']) { diff --git a/packages/form-layout/test/form-layout.test.js b/packages/form-layout/test/form-layout.test.js index 0bbd42d7beb..07520b76ff9 100644 --- a/packages/form-layout/test/form-layout.test.js +++ b/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 = ''; - layout.insertBefore(newFormItem, items[0]); - await nextRender(container); - expect(getComputedStyle(newFormItem).marginLeft).to.be.equal('0px'); + it('should update margin-inline-end after adding
', 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 = ''; - layout.insertBefore(newFormItem, items[0]); - await nextRender(container); + it('should update margin-inline-end after removing
', 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 () => { diff --git a/packages/form-layout/theme/lumo/vaadin-form-layout-styles.js b/packages/form-layout/theme/lumo/vaadin-form-layout-styles.js index 553f79f6d5a..de2f7263fa5 100644 --- a/packages/form-layout/theme/lumo/vaadin-form-layout-styles.js +++ b/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' },