Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] refactor!: initial implementation of form-layout using CSS grid #8590

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3fa920c
refactor: initial implementation of form-layout using CSS grid
DiegoCardoso Jan 29, 2025
d394524
minimize diff
vursen Jan 29, 2025
fae59b7
fix linter errors
vursen Jan 29, 2025
7df07bf
calculate responsive steps with CSS grid
vursen Jan 29, 2025
2c5f25a
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Jan 30, 2025
5e5e075
add support for colspan and line-breaks / clean-ups
DiegoCardoso Jan 29, 2025
1f4aefb
remove obsolete tests
vursen Jan 30, 2025
ceb9bc3
update tests
vursen Jan 30, 2025
bca7528
remove unused import
vursen Jan 30, 2025
167bdcf
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Jan 31, 2025
162f594
remove container queries
vursen Jan 31, 2025
97365f1
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Jan 31, 2025
35567d5
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Jan 31, 2025
f2a2335
minimize diff
vursen Jan 31, 2025
18ce6a9
fix: remove --_grid-colstart when removing <br>
vursen Jan 31, 2025
a5a99a6
fix linter errors
vursen Jan 31, 2025
dfd99f7
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Feb 4, 2025
83dee03
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Feb 4, 2025
1daea8a
add missing nextResize
vursen Feb 4, 2025
c68430f
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Feb 4, 2025
615b832
revert --vaadin-form-item-row-spacing
vursen Feb 4, 2025
7fde63a
Merge remote-tracking branch 'origin/main' into refactor/form-layout-…
vursen Feb 4, 2025
fdb63d4
set --vaadin-form-item-row-spacing to 0 for Material theme
vursen Feb 4, 2025
c54dcc2
test: improve coverage by using native input
vursen Feb 4, 2025
9ddd192
update Material screenshots
vursen Feb 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dev/form-layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0" />
<title>Form Layout</title>
<script type="module" src="./common.js"></script>

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
95 changes: 25 additions & 70 deletions packages/form-layout/src/vaadin-form-layout-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const FormLayoutMixin = (superClass) =>
_columnCount: {
type: Number,
sync: true,
observer: '__columnCountChanged',
},

/**
Expand All @@ -90,14 +91,11 @@ export const FormLayoutMixin = (superClass) =>
_labelsOnTop: {
type: Boolean,
sync: true,
observer: '__labelsOnTopChanged',
},
};
}

static get observers() {
return ['_invokeUpdateLayout(_columnCount, _labelsOnTop)'];
}

/** @protected */
ready() {
// Here we attach a style element that we use for validating
Expand Down Expand Up @@ -256,7 +254,12 @@ export const FormLayoutMixin = (superClass) =>
}

/** @private */
_invokeUpdateLayout() {
__columnCountChanged(columnCount) {
this.$.layout.style.setProperty('--_grid-cols', columnCount);
}

/** @private */
__labelsOnTopChanged() {
this._updateLayout();
}

Expand All @@ -270,75 +273,15 @@ 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) => {
let resetColumn = false;
[...this.children]
.filter((child) => getComputedStyle(child).display !== 'none' || child.localName === 'br')
.forEach((child) => {
if (child.localName === 'br') {
// Reset column count on line break
col = 0;
resetColumn = true;
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;
child.style.width = `calc(${childRatio * 100}% - ${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') {
Expand All @@ -350,6 +293,18 @@ export const FormLayoutMixin = (superClass) =>
child.removeAttribute('label-position');
}
}

const colspan = child.getAttribute('colspan') || child.getAttribute('data-colspan');
if (colspan) {
child.style.setProperty('--_grid-colspan', colspan);
}

if (resetColumn) {
child.style.setProperty('--_grid-colstart', 1);
resetColumn = false;
} else {
child.style.removeProperty('--_grid-colstart');
}
});
}

Expand Down
18 changes: 7 additions & 11 deletions packages/form-layout/src/vaadin-form-layout-styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ export const formLayoutStyles = css`
:host {
display: block;
max-width: 100%;
/* Number of cols, defined by breakpoints. Default value is probably pointless. */
--_grid-cols: 10;
/* CSS API for host */
--vaadin-form-item-label-width: 8em;
--vaadin-form-item-label-spacing: 1em;
--vaadin-form-item-row-spacing: 0;
--vaadin-form-layout-row-spacing: 1em;
--vaadin-form-layout-column-spacing: 2em; /* (default) */
align-self: stretch;
Expand All @@ -22,21 +25,14 @@ export const formLayoutStyles = css`
}

#layout {
display: flex;

display: grid;
grid-template-columns: repeat(var(--_grid-cols), 1fr);
gap: var(--vaadin-form-layout-row-spacing) var(--vaadin-form-layout-column-spacing);
align-items: baseline; /* default \`stretch\` is not appropriate */

flex-wrap: wrap; /* the items should wrap */
}

#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));
grid-column: var(--_grid-colstart, auto) / span min(var(--_grid-colspan, 1), var(--_grid-cols));
}

#layout ::slotted(br) {
Expand Down
128 changes: 35 additions & 93 deletions packages/form-layout/test/form-layout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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';

function estimateEffectiveColumnCount(layout) {
const offsets = [...layout.children]
Expand Down Expand Up @@ -92,17 +91,15 @@ describe('form layout', () => {
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 not have default form layout row spacing', () => {
expect(getComputedStyle(layout.$.layout).rowGap).to.equal('0px');
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', () => {
Expand All @@ -114,48 +111,9 @@ describe('form layout', () => {

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
});
});

describe('colspan', () => {
let layout;

beforeEach(() => {
layout = fixtureSync(`
<vaadin-form-layout responsive-steps='[{"columns": 3}]'>
<vaadin-text-field></vaadin-text-field>
<vaadin-text-field colspan="1"></vaadin-text-field>
<vaadin-text-field colspan="2"></vaadin-text-field>
<vaadin-text-field colspan="3"></vaadin-text-field>
<vaadin-text-field colspan="4"></vaadin-text-field>
<vaadin-text-field colspan="non-number"></vaadin-text-field>
</vaadin-form-layout>
`);
});

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);
expect(getComputedStyle(layout.$.layout).columnGap).to.equal('8px');
});
});

Expand All @@ -182,14 +140,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 }];

Expand Down Expand Up @@ -492,35 +442,31 @@ describe('form layout', () => {
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');
Expand All @@ -535,25 +481,21 @@ describe('form layout', () => {
expect(unhiddenItemWidth).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 grid-column-start after adding <br>', async () => {
const br = document.createElement('br');
layout.insertBefore(br, items[1]);
await nextRender(layout);
expect(getComputedStyle(items[1]).gridColumnStart).to.equal('1');
});

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 grid-column-start 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[1]).gridColumnStart).to.equal('auto');
});

it('should not update layout when setting hidden to true', async () => {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading