-
Notifications
You must be signed in to change notification settings - Fork 86
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
refactor(FormLayout): split layout logic into distinct classes #8770
Changes from 5 commits
94db3f3
cad685d
3ffd7bc
22ec06c
57b5b09
399a2fb
b91c529
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
/** | ||
* @license | ||
* Copyright (c) 2021 - 2025 Vaadin Ltd. | ||
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ | ||
*/ | ||
|
||
/** | ||
* An abstract class for layout controllers. Not intended for public use. | ||
* | ||
* @private | ||
*/ | ||
export class AbstractLayoutController { | ||
constructor(host, config) { | ||
this.host = host; | ||
this.props = {}; | ||
this.config = config; | ||
this.isConnected = false; | ||
|
||
/** @private */ | ||
this.__resizeObserver = new ResizeObserver((entries) => setTimeout(() => this._onResize(entries))); | ||
|
||
/** @private */ | ||
this.__mutationObserver = new MutationObserver((entries) => this._onMutation(entries)); | ||
} | ||
|
||
/** | ||
* Connects the layout to the host element. | ||
*/ | ||
connect() { | ||
if (this.isConnected) { | ||
return; | ||
} | ||
|
||
this.isConnected = true; | ||
this.__resizeObserver.observe(this.host); | ||
this.__mutationObserver.observe(this.host, this.config.mutationObserverOptions); | ||
} | ||
|
||
/** | ||
* Disconnects the layout from the host element. | ||
*/ | ||
disconnect() { | ||
if (!this.isConnected) { | ||
return; | ||
} | ||
|
||
this.isConnected = false; | ||
this.__resizeObserver.disconnect(); | ||
this.__mutationObserver.disconnect(); | ||
} | ||
|
||
/** | ||
* Sets the properties of the layout controller. | ||
*/ | ||
setProps(props) { | ||
this.props = props; | ||
} | ||
|
||
/** | ||
* Updates the layout based on the current properties. | ||
*/ | ||
updateLayout() { | ||
// To be implemented | ||
} | ||
|
||
/** @protected */ | ||
_onResize(_entries) { | ||
// To be implemented | ||
} | ||
|
||
/** @protected */ | ||
_onMutation(_entries) { | ||
// To be implemented | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,177 @@ | ||
/** | ||
* @license | ||
* Copyright (c) 2021 - 2025 Vaadin Ltd. | ||
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ | ||
*/ | ||
import { isElementHidden } from '@vaadin/a11y-base/src/focus-utils'; | ||
import { AbstractLayoutController } from './abstract-layout-controller.js'; | ||
|
||
/** | ||
* Check if the node is a line break element. | ||
* | ||
* @param {HTMLElement} el | ||
* @return {boolean} | ||
*/ | ||
function isBreakLine(el) { | ||
return el.localName === 'br'; | ||
} | ||
|
||
/** | ||
* A controller that implements the auto-responsive layout algorithm. | ||
* Not intended for public use. | ||
* | ||
* @private | ||
*/ | ||
export class AutoResponsiveLayoutController extends AbstractLayoutController { | ||
constructor(host) { | ||
super(host, { | ||
mutationObserverOptions: { | ||
subtree: true, | ||
childList: true, | ||
attributes: true, | ||
attributeFilter: ['colspan', 'data-colspan', 'hidden'], | ||
}, | ||
}); | ||
} | ||
|
||
/** @override */ | ||
connect() { | ||
if (this.isConnected) { | ||
return; | ||
} | ||
|
||
super.connect(); | ||
|
||
this.updateLayout(); | ||
} | ||
|
||
/** @override */ | ||
disconnect() { | ||
web-padawan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!this.isConnected) { | ||
return; | ||
} | ||
|
||
super.disconnect(); | ||
|
||
const { host } = this; | ||
host.style.removeProperty('--_column-width'); | ||
host.style.removeProperty('--_max-columns'); | ||
host.$.layout.removeAttribute('fits-labels-aside'); | ||
host.$.layout.style.removeProperty('--_grid-rendered-column-count'); | ||
|
||
this.__children.forEach((child) => { | ||
child.style.removeProperty('--_grid-colstart'); | ||
child.style.removeProperty('--_grid-colspan'); | ||
}); | ||
} | ||
|
||
/** @override */ | ||
setProps(props) { | ||
super.setProps(props); | ||
|
||
if (this.isConnected) { | ||
this.updateLayout(); | ||
} | ||
} | ||
|
||
/** @override */ | ||
updateLayout() { | ||
const { host, props } = this; | ||
if (!this.isConnected || isElementHidden(host)) { | ||
return; | ||
} | ||
|
||
let columnCount = 0; | ||
let maxColumns = 0; | ||
|
||
const children = this.__children; | ||
children | ||
.filter((child) => isBreakLine(child) || !isElementHidden(child)) | ||
.forEach((child, index, children) => { | ||
const prevChild = children[index - 1]; | ||
|
||
if (isBreakLine(child)) { | ||
columnCount = 0; | ||
return; | ||
} | ||
|
||
if ( | ||
(prevChild && prevChild.parentElement !== child.parentElement) || | ||
(!props.autoRows && child.parentElement === host) | ||
) { | ||
columnCount = 0; | ||
} | ||
|
||
if (props.autoRows && columnCount === 0) { | ||
child.style.setProperty('--_grid-colstart', 1); | ||
} else { | ||
child.style.removeProperty('--_grid-colstart'); | ||
} | ||
|
||
const colspan = child.getAttribute('colspan') || child.getAttribute('data-colspan'); | ||
if (colspan) { | ||
columnCount += parseInt(colspan); | ||
child.style.setProperty('--_grid-colspan', colspan); | ||
} else { | ||
columnCount += 1; | ||
child.style.removeProperty('--_grid-colspan'); | ||
} | ||
|
||
maxColumns = Math.max(maxColumns, columnCount); | ||
}); | ||
|
||
children.filter(isElementHidden).forEach((child) => { | ||
child.style.removeProperty('--_grid-colstart'); | ||
}); | ||
|
||
host.style.setProperty('--_column-width', props.columnWidth); | ||
host.style.setProperty('--_max-columns', Math.min(props.maxColumns, maxColumns)); | ||
|
||
host.$.layout.toggleAttribute('fits-labels-aside', this.props.labelsAside && this.__fitsLabelsAside); | ||
host.$.layout.style.setProperty('--_grid-rendered-column-count', this.__renderedColumnCount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to revert to the previous approach that may trigger an extra style recalculation due to reading getComputedStyle after setting CSS properties. Without this, some tests failed – apparently, they were passing before due to a different order of observers, which was actually not efficient. |
||
} | ||
|
||
/** @override */ | ||
_onResize() { | ||
this.updateLayout(); | ||
} | ||
|
||
/** @override */ | ||
_onMutation(entries) { | ||
const shouldUpdateLayout = entries.some(({ target }) => { | ||
return ( | ||
target === this.host || | ||
target.parentElement === this.host || | ||
target.parentElement.localName === 'vaadin-form-row' | ||
); | ||
}); | ||
if (shouldUpdateLayout) { | ||
this.updateLayout(); | ||
} | ||
} | ||
|
||
/** @private */ | ||
get __children() { | ||
return [...this.host.children].flatMap((child) => { | ||
return child.localName === 'vaadin-form-row' ? [...child.children] : child; | ||
}); | ||
} | ||
|
||
/** @private */ | ||
get __renderedColumnCount() { | ||
// Calculate the number of rendered columns, excluding CSS grid auto columns (0px) | ||
const { gridTemplateColumns } = getComputedStyle(this.host.$.layout); | ||
return gridTemplateColumns.split(' ').filter((width) => width !== '0px').length; | ||
} | ||
|
||
/** @private */ | ||
get __columnWidthWithLabelsAside() { | ||
const { backgroundPositionY } = getComputedStyle(this.host.$.layout, '::before'); | ||
return parseFloat(backgroundPositionY); | ||
} | ||
|
||
/** @private */ | ||
get __fitsLabelsAside() { | ||
return this.host.offsetWidth >= this.__columnWidthWithLabelsAside; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be beneficial to add a JSdoc type definition here to help with autocomplete?
Something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.