From 3fa920c4d4dd587abf5e616fc2e8141b67a71c4b Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Wed, 29 Jan 2025 11:39:15 +0200 Subject: [PATCH 01/12] refactor: initial implementation of form-layout using CSS grid --- .../src/vaadin-form-layout-styles.js | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index dee1af4a95..1f1ebccae3 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -10,12 +10,16 @@ export const formLayoutStyles = css` display: block; max-width: 100%; animation: 1ms vaadin-form-layout-appear; + align-self: stretch; + container-type: inline-size; + container-name: form-grid; + --_grid-cols: 10; /* Number of cols, defined by breakpoints. Default value is probably pointless. */ /* CSS API for host */ + /* Let's not define defaults here – that way they can be set globally */ --vaadin-form-item-label-width: 8em; --vaadin-form-item-label-spacing: 1em; --vaadin-form-item-row-spacing: 1em; - --vaadin-form-layout-column-spacing: 2em; /* (default) */ - align-self: stretch; + --vaadin-form-layout-column-spacing: 2em; } @keyframes vaadin-form-layout-appear { @@ -29,21 +33,20 @@ export const formLayoutStyles = css` } #layout { - display: flex; - - align-items: baseline; /* default \`stretch\` is not appropriate */ - - flex-wrap: wrap; /* the items should wrap */ + display: grid; + grid-template-columns: repeat(var(--_grid-cols), 1fr); + column-gap: var(--vaadin-form-layout-column-spacing); + row-gap: var(--vaadin-form-item-row-spacing); + align-items: baseline; + justify-content: stretch; + min-width: min-content; } #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)); + /* Unless auto-rows is on, each field starts on a new line */ + grid-column-start: 1; + /* Span is set via custom prop set by the WC; capped by current breakpoint's column count */ + grid-column-end: span min(var(--vaadin-form-layout-colspan), var(--_grid-cols)); } #layout ::slotted(br) { @@ -56,7 +59,6 @@ export const formItemStyles = css` display: inline-flex; flex-direction: row; align-items: baseline; - margin: calc(0.5 * var(--vaadin-form-item-row-spacing, 1em)) 0; } :host([label-position='top']) { From d394524f722a6347ed7f050f854e59e0b6317830 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 29 Jan 2025 14:28:43 +0400 Subject: [PATCH 02/12] minimize diff --- packages/form-layout/src/vaadin-form-layout-styles.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index 1f1ebccae3..94e44d5ac8 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -10,16 +10,16 @@ export const formLayoutStyles = css` display: block; max-width: 100%; animation: 1ms vaadin-form-layout-appear; - align-self: stretch; container-type: inline-size; container-name: form-grid; - --_grid-cols: 10; /* Number of cols, defined by breakpoints. Default value is probably pointless. */ + /* Number of cols, defined by breakpoints. Default value is probably pointless. */ + --_grid-cols: 10; /* CSS API for host */ - /* Let's not define defaults here – that way they can be set globally */ --vaadin-form-item-label-width: 8em; --vaadin-form-item-label-spacing: 1em; --vaadin-form-item-row-spacing: 1em; - --vaadin-form-layout-column-spacing: 2em; + --vaadin-form-layout-column-spacing: 2em; /* (default) */ + align-self: stretch; } @keyframes vaadin-form-layout-appear { @@ -37,7 +37,7 @@ export const formLayoutStyles = css` grid-template-columns: repeat(var(--_grid-cols), 1fr); column-gap: var(--vaadin-form-layout-column-spacing); row-gap: var(--vaadin-form-item-row-spacing); - align-items: baseline; + align-items: baseline; /* default \`stretch\` is not appropriate */ justify-content: stretch; min-width: min-content; } From fae59b7a4e46ba7efe08215d45c9fb809f24ea54 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 29 Jan 2025 14:30:15 +0400 Subject: [PATCH 03/12] fix linter errors --- packages/form-layout/src/vaadin-form-layout-styles.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index 94e44d5ac8..e92cf2a92d 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -35,18 +35,14 @@ export const formLayoutStyles = css` #layout { display: grid; grid-template-columns: repeat(var(--_grid-cols), 1fr); - column-gap: var(--vaadin-form-layout-column-spacing); - row-gap: var(--vaadin-form-item-row-spacing); + gap: var(--vaadin-form-item-row-spacing) var(--vaadin-form-layout-column-spacing); align-items: baseline; /* default \`stretch\` is not appropriate */ justify-content: stretch; min-width: min-content; } #layout ::slotted(*) { - /* Unless auto-rows is on, each field starts on a new line */ - grid-column-start: 1; - /* Span is set via custom prop set by the WC; capped by current breakpoint's column count */ - grid-column-end: span min(var(--vaadin-form-layout-colspan), var(--_grid-cols)); + grid-column: 1 / span min(var(--vaadin-form-layout-colspan), var(--_grid-cols)); } #layout ::slotted(br) { From 7df07bf8db464d11581349840f194901ef09c110 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 29 Jan 2025 16:35:29 +0400 Subject: [PATCH 04/12] calculate responsive steps with CSS grid --- dev/form-layout.html | 2 +- .../src/vaadin-form-layout-mixin.js | 194 ++++++++++-------- .../src/vaadin-form-layout-styles.js | 2 +- 3 files changed, 116 insertions(+), 82 deletions(-) diff --git a/dev/form-layout.html b/dev/form-layout.html index 6b0870d697..07aa94c453 100644 --- a/dev/form-layout.html +++ b/dev/form-layout.html @@ -3,7 +3,7 @@ - + Form Layout diff --git a/packages/form-layout/src/vaadin-form-layout-mixin.js b/packages/form-layout/src/vaadin-form-layout-mixin.js index 536687a97c..e8c9357710 100644 --- a/packages/form-layout/src/vaadin-form-layout-mixin.js +++ b/packages/form-layout/src/vaadin-form-layout-mixin.js @@ -81,6 +81,7 @@ export const FormLayoutMixin = (superClass) => _columnCount: { type: Number, sync: true, + observer: '__columnCountChanged', }, /** @@ -195,6 +196,7 @@ export const FormLayoutMixin = (superClass) => }); if (addedNodes.length > 0 || removedNodes.length > 0) { + // TODO: Optimize this to only update layout if a resize happened this._updateLayout(); } }); @@ -322,6 +324,11 @@ export const FormLayoutMixin = (superClass) => } } + /** @private */ + __columnCountChanged(columnCount) { + this.$.layout.style.setProperty('--_grid-cols', columnCount); + } + /** @private */ _invokeUpdateLayout() { this._updateLayout(); @@ -337,90 +344,117 @@ export const FormLayoutMixin = (superClass) => return; } - /* - The item width formula: - - itemWidth = colspan / columnCount * 100% - columnSpacing - - We have to subtract columnSpacing, because the column spacing space is taken - by item margins of 1/2 * spacing on both sides - */ - - 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; - Array.from(this.children) - .filter((child) => child.localName === 'br' || getComputedStyle(child).display !== 'none') - .forEach((child, index, children) => { - if (child.localName === 'br') { - // Reset column count on line break - col = 0; - return; - } - - const attrColspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); - let colspan; - colspan = this._naturalNumberOrOne(parseFloat(attrColspan)); - - // Never span further than the number of columns - colspan = Math.min(colspan, this._columnCount); - - const childRatio = colspan / this._columnCount; - - // Note: using 99.9% for 100% fixes rounding errors in MS Edge - // (< v16), otherwise the items might wrap, resizing is wobbly. - child.style.width = `calc(${childRatio * 99.9}% - ${1 - childRatio} * ${columnSpacing})`; - - if (col + colspan > this._columnCount) { - // Too big to fit on this row, let's wrap it - col = 0; - } + [...this.children].forEach((child) => { + const colspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); + if (colspan) { + child.style.setProperty('--_grid-colspan', colspan); + } - // At the start edge - if (col === 0) { - child.style.setProperty(marginStartProp, '0px'); - } else { - child.style.removeProperty(marginStartProp); + if (child.localName === 'vaadin-form-item') { + if (this._labelsOnTop) { + if (child.getAttribute('label-position') !== 'top') { + child.__useLayoutLabelPosition = true; + child.setAttribute('label-position', 'top'); + } + } else if (child.__useLayoutLabelPosition) { + delete child.__useLayoutLabelPosition; + child.removeAttribute('label-position'); } + } + }); - 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) { - const colspanRatio = (this._columnCount - col - colspan) / this._columnCount; - child.style.setProperty( - marginEndProp, - `calc(${colspanRatio * containerWidth}px + ${colspanRatio} * ${columnSpacing})`, - ); - } else { - child.style.removeProperty(marginEndProp); - } + // /* + // The item width formula: + + // itemWidth = colspan / columnCount * 100% - columnSpacing + + // We have to subtract columnSpacing, because the column spacing space is taken + // by item margins of 1/2 * spacing on both sides + // */ + + // 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; + // Array.from(this.children) + // .filter((child) => child.localName === 'br' || getComputedStyle(child).display !== 'none') + // .forEach((child, index, children) => { + // if (child.localName === 'br') { + // // Reset column count on line break + // col = 0; + // return; + // } + + // const attrColspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); + // let colspan; + // colspan = this._naturalNumberOrOne(parseFloat(attrColspan)); + + // // Never span further than the number of columns + // colspan = Math.min(colspan, this._columnCount); + + // const childRatio = colspan / this._columnCount; + + // // Note: using 99.9% for 100% fixes rounding errors in MS Edge + // // (< v16), otherwise the items might wrap, resizing is wobbly. + // child.style.width = `calc(${childRatio * 99.9}% - ${1 - childRatio} * ${columnSpacing})`; + + // if (col + colspan > this._columnCount) { + // // Too big to fit on this row, let's wrap it + // 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) { + // const colspanRatio = (this._columnCount - col - colspan) / this._columnCount; + // child.style.setProperty( + // marginEndProp, + // `calc(${colspanRatio * containerWidth}px + ${colspanRatio} * ${columnSpacing})`, + // ); + // } else { + // child.style.removeProperty(marginEndProp); + // } + + // // Move the column counter + // col = (col + colspan) % this._columnCount; + + // if (child.localName === 'vaadin-form-item') { + // if (this._labelsOnTop) { + // if (child.getAttribute('label-position') !== 'top') { + // child.__useLayoutLabelPosition = true; + // child.setAttribute('label-position', 'top'); + // } + // } else if (child.__useLayoutLabelPosition) { + // delete child.__useLayoutLabelPosition; + // child.removeAttribute('label-position'); + // } + // } + // }); + } - // Move the column counter - col = (col + colspan) % this._columnCount; - - if (child.localName === 'vaadin-form-item') { - if (this._labelsOnTop) { - if (child.getAttribute('label-position') !== 'top') { - child.__useLayoutLabelPosition = true; - child.setAttribute('label-position', 'top'); - } - } else if (child.__useLayoutLabelPosition) { - delete child.__useLayoutLabelPosition; - child.removeAttribute('label-position'); - } - } - }); + /** @private */ + __generateContainerQueries() { + // TODO: Might use adoptedStyleSheets when supported? + // NOTE: Container queries are not worth using because + // we need to support [label-position] attribute on form-item + // which users can target in their styles. } /** diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index e92cf2a92d..7742e9955e 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -42,7 +42,7 @@ export const formLayoutStyles = css` } #layout ::slotted(*) { - grid-column: 1 / span min(var(--vaadin-form-layout-colspan), var(--_grid-cols)); + grid-column-end: span min(var(--_grid-colspan, 1), var(--_grid-cols)); } #layout ::slotted(br) { From 5e5e075bbdf76a7d4a9f8e2730c2da9b9d955b42 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Wed, 29 Jan 2025 16:29:33 +0200 Subject: [PATCH 05/12] add support for colspan and line-breaks / clean-ups --- .../src/vaadin-form-layout-mixin.js | 140 ++++-------------- .../src/vaadin-form-layout-styles.js | 1 + 2 files changed, 29 insertions(+), 112 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-mixin.js b/packages/form-layout/src/vaadin-form-layout-mixin.js index e8c9357710..51d95e4f7a 100644 --- a/packages/form-layout/src/vaadin-form-layout-mixin.js +++ b/packages/form-layout/src/vaadin-form-layout-mixin.js @@ -91,6 +91,7 @@ export const FormLayoutMixin = (superClass) => _labelsOnTop: { type: Boolean, sync: true, + observer: '__labelsOnTopChanged', }, /** @private */ @@ -100,10 +101,6 @@ export const FormLayoutMixin = (superClass) => }; } - static get observers() { - return ['_invokeUpdateLayout(_columnCount, _labelsOnTop)']; - } - /** @protected */ ready() { // Here we attach a style element that we use for validating @@ -196,7 +193,6 @@ export const FormLayoutMixin = (superClass) => }); if (addedNodes.length > 0 || removedNodes.length > 0) { - // TODO: Optimize this to only update layout if a resize happened this._updateLayout(); } }); @@ -330,7 +326,7 @@ export const FormLayoutMixin = (superClass) => } /** @private */ - _invokeUpdateLayout() { + __labelsOnTopChanged() { this._updateLayout(); } @@ -344,117 +340,37 @@ export const FormLayoutMixin = (superClass) => return; } - [...this.children].forEach((child) => { - const colspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); - if (colspan) { - child.style.setProperty('--_grid-colspan', colspan); - } + let resetColumn = false; + [...this.children] + .filter((child) => getComputedStyle(child).display !== 'none' || child.localName === 'br') + .forEach((child) => { + if (child.localName === 'br') { + resetColumn = true; + return; + } - if (child.localName === 'vaadin-form-item') { - if (this._labelsOnTop) { - if (child.getAttribute('label-position') !== 'top') { - child.__useLayoutLabelPosition = true; - child.setAttribute('label-position', 'top'); + if (child.localName === 'vaadin-form-item') { + if (this._labelsOnTop) { + if (child.getAttribute('label-position') !== 'top') { + child.__useLayoutLabelPosition = true; + child.setAttribute('label-position', 'top'); + } + } else if (child.__useLayoutLabelPosition) { + delete child.__useLayoutLabelPosition; + child.removeAttribute('label-position'); } - } else if (child.__useLayoutLabelPosition) { - delete child.__useLayoutLabelPosition; - child.removeAttribute('label-position'); } - } - }); - // /* - // The item width formula: - - // itemWidth = colspan / columnCount * 100% - columnSpacing - - // We have to subtract columnSpacing, because the column spacing space is taken - // by item margins of 1/2 * spacing on both sides - // */ - - // 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; - // Array.from(this.children) - // .filter((child) => child.localName === 'br' || getComputedStyle(child).display !== 'none') - // .forEach((child, index, children) => { - // if (child.localName === 'br') { - // // Reset column count on line break - // col = 0; - // return; - // } - - // const attrColspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); - // let colspan; - // colspan = this._naturalNumberOrOne(parseFloat(attrColspan)); - - // // Never span further than the number of columns - // colspan = Math.min(colspan, this._columnCount); - - // const childRatio = colspan / this._columnCount; - - // // Note: using 99.9% for 100% fixes rounding errors in MS Edge - // // (< v16), otherwise the items might wrap, resizing is wobbly. - // child.style.width = `calc(${childRatio * 99.9}% - ${1 - childRatio} * ${columnSpacing})`; - - // if (col + colspan > this._columnCount) { - // // Too big to fit on this row, let's wrap it - // 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) { - // const colspanRatio = (this._columnCount - col - colspan) / this._columnCount; - // child.style.setProperty( - // marginEndProp, - // `calc(${colspanRatio * containerWidth}px + ${colspanRatio} * ${columnSpacing})`, - // ); - // } else { - // child.style.removeProperty(marginEndProp); - // } - - // // Move the column counter - // col = (col + colspan) % this._columnCount; - - // if (child.localName === 'vaadin-form-item') { - // if (this._labelsOnTop) { - // if (child.getAttribute('label-position') !== 'top') { - // child.__useLayoutLabelPosition = true; - // child.setAttribute('label-position', 'top'); - // } - // } else if (child.__useLayoutLabelPosition) { - // delete child.__useLayoutLabelPosition; - // child.removeAttribute('label-position'); - // } - // } - // }); - } + const colspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); + if (colspan) { + child.style.setProperty('--_grid-colspan', colspan); + } - /** @private */ - __generateContainerQueries() { - // TODO: Might use adoptedStyleSheets when supported? - // NOTE: Container queries are not worth using because - // we need to support [label-position] attribute on form-item - // which users can target in their styles. + if (resetColumn) { + child.style.setProperty('--_grid-colstart', 1); + resetColumn = false; + } + }); } /** diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index 7742e9955e..f21a53f815 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -42,6 +42,7 @@ export const formLayoutStyles = css` } #layout ::slotted(*) { + grid-column-start: var(--_grid-colstart, auto); grid-column-end: span min(var(--_grid-colspan, 1), var(--_grid-cols)); } From 1f4aefba43586ce299571aa042c2ceac1079e078 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Thu, 30 Jan 2025 10:13:47 +0400 Subject: [PATCH 06/12] remove obsolete tests --- .../form-layout/test/form-layout.common.js | 46 ------------------- 1 file changed, 46 deletions(-) diff --git a/packages/form-layout/test/form-layout.common.js b/packages/form-layout/test/form-layout.common.js index 280ab3056a..c56f16e3b2 100644 --- a/packages/form-layout/test/form-layout.common.js +++ b/packages/form-layout/test/form-layout.common.js @@ -123,52 +123,6 @@ describe('form layout', () => { expect(parseFloat(getComputedStyle(item).marginTop)).to.equal(0); expect(parseFloat(getComputedStyle(item).marginBottom)).to.equal(0); }); - - 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 - }); - }); - - describe('colspan', () => { - let layout; - - beforeEach(() => { - layout = fixtureSync(` - - - - - - - - - `); - }); - - function estimateEffectiveColspan(el) { - return parseFloat(getParsedWidth(el).percentage) / (100 / 3); - } - - it('should span children correctly', () => { - // Empty means 1 - expect(estimateEffectiveColspan(layout.children[0])).to.be.closeTo(1, 0.1); - - // Correct values - expect(estimateEffectiveColspan(layout.children[1])).to.be.closeTo(1, 0.1); - expect(estimateEffectiveColspan(layout.children[2])).to.be.closeTo(2, 0.1); - expect(estimateEffectiveColspan(layout.children[3])).to.be.closeTo(3, 0.1); - - // If more then a number of columns, use number of columns - expect(estimateEffectiveColspan(layout.children[4])).to.be.closeTo(3, 0.1); - - // Invalid means 1 - expect(estimateEffectiveColspan(layout.children[5])).to.be.closeTo(1, 0.1); - }); }); describe('container overflow', () => { From ceb9bc3733aae9d88b09f33e184136b4a288c57f Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Thu, 30 Jan 2025 16:10:53 +0400 Subject: [PATCH 07/12] update tests --- .../form-layout/test/form-layout.common.js | 133 ++++++------------ 1 file changed, 44 insertions(+), 89 deletions(-) diff --git a/packages/form-layout/test/form-layout.common.js b/packages/form-layout/test/form-layout.common.js index c56f16e3b2..89f66f2a59 100644 --- a/packages/form-layout/test/form-layout.common.js +++ b/packages/form-layout/test/form-layout.common.js @@ -2,42 +2,6 @@ import { expect } from '@vaadin/chai-plugins'; import { aTimeout, fixtureSync, nextFrame, nextRender, nextResize } from '@vaadin/testing-helpers'; import sinon from 'sinon'; import '@polymer/polymer/lib/elements/dom-repeat.js'; -import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; - -customElements.define( - 'mutable-layout', - class extends PolymerElement { - static get template() { - return html` - - - - - - - - - - - - `; - } - - static get properties() { - return { - items: { - type: Array, - value: () => [], - }, - }; - } - }, -); function getParsedWidth(el) { const width = el.style.getPropertyValue('width'); @@ -221,14 +185,6 @@ describe('form layout', () => { ]); }); - it('should assign width inline style on items', () => { - layout.responsiveSteps = [{ columns: 3 }]; - - const parsedWidth = getParsedWidth(layout.firstElementChild); - expect(parsedWidth.percentage).to.match(/%$/u); - expect(parseFloat(parsedWidth.percentage)).to.be.closeTo(33, 0.5); - }); - it('should set label-position attribute to child form-item elements', () => { layout.responsiveSteps = [{ columns: 1 }]; @@ -548,43 +504,53 @@ describe('form layout', () => { }); describe('mutations', () => { - let container, layout; + let container, layout, items; beforeEach(async () => { - container = fixtureSync(''); - layout = container.shadowRoot.querySelector('vaadin-form-layout'); + container = fixtureSync(` +
+ + + + + + + + + + +
+ `); + layout = container.querySelector('vaadin-form-layout'); + items = layout.querySelectorAll('vaadin-form-item'); await nextRender(container); }); - function estimateEffectiveColspan(el) { - return parseFloat(getParsedWidth(el).percentage) / (100 / 2); - } + it('should update grid-column-end after updating a colspan attribute', async () => { + expect(getComputedStyle(items[0]).gridColumnEnd).to.equal('span 1'); - it('should update layout after updating a colspan attribute', async () => { - expect(estimateEffectiveColspan(layout.children[0])).to.be.closeTo(1, 0.1); - - layout.children[0].setAttribute('colspan', 2); - await nextRender(container); - expect(estimateEffectiveColspan(layout.children[0])).to.be.closeTo(2, 0.1); + items[0].setAttribute('colspan', 2); + await nextRender(layout); + expect(getComputedStyle(items[0]).gridColumnEnd).to.equal('span 2'); }); - it('should update layout after updating a data-colspan attribute', async () => { - expect(estimateEffectiveColspan(layout.children[0])).to.be.closeTo(1, 0.1); + it('should update grid-column-end after updating a data-colspan attribute', async () => { + expect(getComputedStyle(items[0]).gridColumnEnd).to.equal('span 1'); - layout.children[0].setAttribute('data-colspan', 2); - await nextRender(container); - expect(estimateEffectiveColspan(layout.children[0])).to.be.closeTo(2, 0.1); + items[0].setAttribute('data-colspan', 2); + await nextRender(layout); + expect(getComputedStyle(items[0]).gridColumnEnd).to.equal('span 2'); }); it('should prefer colspan attribute over data-colspan when both are set', async () => { - layout.children[0].setAttribute('colspan', 2); - layout.children[0].setAttribute('data-colspan', 1); - await nextRender(container); - expect(estimateEffectiveColspan(layout.children[0])).to.be.closeTo(2, 0.1); + items[0].setAttribute('colspan', 2); + items[0].setAttribute('data-colspan', 1); + await nextRender(layout); + expect(getComputedStyle(items[0]).gridColumnEnd).to.equal('span 2'); }); it('should update style if hidden property of layout-item is changed and the element has not had style yet', async () => { - const itemWidth = layout.children[0].getBoundingClientRect().width; + const itemWidth = items[0].getBoundingClientRect().width; expect(itemWidth).to.be.above(0); const newFormItem = document.createElement('vaadin-form-item'); @@ -599,32 +565,21 @@ describe('form layout', () => { expect(unhiddenItemWidth).to.equal(itemWidth); }); - it('should update layout after updating a colspan attribute on the lazily stamped node', async () => { - container.push('items', { label: 'Email', colspan: 1 }); - await nextRender(container); - const item = layout.querySelectorAll('vaadin-form-item')[2]; - expect(estimateEffectiveColspan(item)).to.be.closeTo(1, 0.1); - - container.set('items.0.colspan', 2); - await nextRender(container); - expect(estimateEffectiveColspan(item)).to.be.closeTo(2, 0.1); + it('should update grid-column-start after adding
', async () => { + const br = document.createElement('br'); + items[0].after(br); + await nextRender(layout); + expect(getComputedStyle(items[1]).gridColumnStart).to.equal('1'); }); - it('should update layout after a new item is added', async () => { - const newFormItem = document.createElement('vaadin-form-item'); - newFormItem.innerHTML = ''; - layout.appendChild(newFormItem); - await nextRender(container); - expect(getComputedStyle(newFormItem).marginLeft).to.be.equal('0px'); - }); - - it('should update layout after an item is removed', async () => { - const itemsList = layout.querySelectorAll('vaadin-form-item'); - expect(getComputedStyle(itemsList[1]).marginLeft).to.not.be.equal('0px'); + it('should update grid-column-start after removing
', async () => { + const br = document.createElement('br'); + items[0].after(br); + await nextRender(layout); - layout.removeChild(itemsList[0]); - await nextRender(container); - expect(getComputedStyle(itemsList[1]).marginLeft).to.be.equal('0px'); + br.remove(); + await nextRender(layout); + expect(getComputedStyle(items[1]).gridColumnStart).to.equal('auto'); }); it('should not update layout when setting hidden to true', async () => { From bca7528c1e6b27f75198b21446051cef6fbb957d Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Thu, 30 Jan 2025 16:14:01 +0400 Subject: [PATCH 08/12] remove unused import --- packages/form-layout/test/form-layout.common.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/form-layout/test/form-layout.common.js b/packages/form-layout/test/form-layout.common.js index 89f66f2a59..aa7f46514b 100644 --- a/packages/form-layout/test/form-layout.common.js +++ b/packages/form-layout/test/form-layout.common.js @@ -1,7 +1,6 @@ import { expect } from '@vaadin/chai-plugins'; import { aTimeout, fixtureSync, nextFrame, nextRender, nextResize } from '@vaadin/testing-helpers'; import sinon from 'sinon'; -import '@polymer/polymer/lib/elements/dom-repeat.js'; function getParsedWidth(el) { const width = el.style.getPropertyValue('width'); From 162f594ca6c77a3e1ce9069e456b8fe9231f2e0a Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 31 Jan 2025 17:47:40 +0400 Subject: [PATCH 09/12] remove container queries --- packages/form-layout/src/vaadin-form-layout-styles.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index f21a53f815..96a48b7881 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -10,8 +10,6 @@ export const formLayoutStyles = css` display: block; max-width: 100%; animation: 1ms vaadin-form-layout-appear; - container-type: inline-size; - container-name: form-grid; /* Number of cols, defined by breakpoints. Default value is probably pointless. */ --_grid-cols: 10; /* CSS API for host */ From f2a233531928a93770368e27b753fb388121f99c Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 31 Jan 2025 18:41:16 +0400 Subject: [PATCH 10/12] minimize diff --- packages/form-layout/src/vaadin-form-layout-styles.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index 96a48b7881..fde20824be 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -35,8 +35,6 @@ export const formLayoutStyles = css` grid-template-columns: repeat(var(--_grid-cols), 1fr); gap: var(--vaadin-form-item-row-spacing) var(--vaadin-form-layout-column-spacing); align-items: baseline; /* default \`stretch\` is not appropriate */ - justify-content: stretch; - min-width: min-content; } #layout ::slotted(*) { From 18ce6a91194a987aaab45b82b62b2dfd21691bff Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 31 Jan 2025 18:46:33 +0400 Subject: [PATCH 11/12] fix: remove --_grid-colstart when removing
--- packages/form-layout/src/vaadin-form-layout-mixin.js | 2 ++ packages/form-layout/test/form-layout.test.js | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-mixin.js b/packages/form-layout/src/vaadin-form-layout-mixin.js index aef44b7a79..cd9fb732d2 100644 --- a/packages/form-layout/src/vaadin-form-layout-mixin.js +++ b/packages/form-layout/src/vaadin-form-layout-mixin.js @@ -374,6 +374,8 @@ export const FormLayoutMixin = (superClass) => if (resetColumn) { child.style.setProperty('--_grid-colstart', 1); resetColumn = false; + } else { + child.style.removeProperty('--_grid-colstart'); } }); } diff --git a/packages/form-layout/test/form-layout.test.js b/packages/form-layout/test/form-layout.test.js index 8ba71f0383..45ba7619d4 100644 --- a/packages/form-layout/test/form-layout.test.js +++ b/packages/form-layout/test/form-layout.test.js @@ -519,14 +519,14 @@ describe('form layout', () => { it('should update grid-column-start after adding
', async () => { const br = document.createElement('br'); - items[0].after(br); + layout.insertBefore(br, items[1]); await nextRender(layout); expect(getComputedStyle(items[1]).gridColumnStart).to.equal('1'); }); it('should update grid-column-start after removing
', async () => { const br = document.createElement('br'); - items[0].after(br); + layout.insertBefore(br, items[1]); await nextRender(layout); br.remove(); From a5a99a64f71a3d1560758e2974b862badc0ee39a Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 31 Jan 2025 18:54:15 +0400 Subject: [PATCH 12/12] fix linter errors --- packages/form-layout/src/vaadin-form-layout-styles.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/form-layout/src/vaadin-form-layout-styles.js b/packages/form-layout/src/vaadin-form-layout-styles.js index fde20824be..80d8c539c8 100644 --- a/packages/form-layout/src/vaadin-form-layout-styles.js +++ b/packages/form-layout/src/vaadin-form-layout-styles.js @@ -38,8 +38,7 @@ export const formLayoutStyles = css` } #layout ::slotted(*) { - grid-column-start: var(--_grid-colstart, auto); - grid-column-end: span min(var(--_grid-colspan, 1), var(--_grid-cols)); + grid-column: var(--_grid-colstart, auto) / span min(var(--_grid-colspan, 1), var(--_grid-cols)); } #layout ::slotted(br) {