Skip to content

Commit 695aefd

Browse files
vursenvursen
committed
refactor!: prevent app-layout from thrashing layout on every resize
1 parent 8a4d0ea commit 695aefd

File tree

12 files changed

+227
-128
lines changed

12 files changed

+227
-128
lines changed

‎packages/app-layout/src/vaadin-app-layout-mixin.js‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,24 @@ import { AriaModalController } from '@vaadin/a11y-base/src/aria-modal-controller
77
import { FocusTrapController } from '@vaadin/a11y-base/src/focus-trap-controller.js';
88
import { isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
99
import { animationFrame } from '@vaadin/component-base/src/async.js';
10+
import { CSSPropertyObserver } from '@vaadin/component-base/src/css-property-observer.js';
1011
import { Debouncer } from '@vaadin/component-base/src/debounce.js';
1112
import { I18nMixin } from '@vaadin/component-base/src/i18n-mixin.js';
1213

14+
CSS.registerProperty({
15+
name: '--vaadin-app-layout-touch-optimized',
16+
syntax: 'true | false',
17+
inherits: true,
18+
initialValue: 'false',
19+
});
20+
21+
CSS.registerProperty({
22+
name: '--vaadin-app-layout-drawer-overlay',
23+
syntax: 'true | false',
24+
inherits: true,
25+
initialValue: 'false',
26+
});
27+
1328
/**
1429
* @typedef {import('./vaadin-app-layout.js').AppLayoutI18n} AppLayoutI18n
1530
*/
@@ -180,6 +195,16 @@ export const AppLayoutMixin = (superclass) =>
180195
this.addController(this.__focusTrapController);
181196
this.__setAriaExpanded();
182197

198+
this.__cssPropertyObserver = new CSSPropertyObserver(this.$.cssPropertyObserver, (propertyName) => {
199+
if (propertyName === '--vaadin-app-layout-touch-optimized') {
200+
this._updateTouchOptimizedMode();
201+
}
202+
if (propertyName === '--vaadin-app-layout-drawer-overlay') {
203+
this._updateOverlayMode();
204+
}
205+
});
206+
this.__cssPropertyObserver.observe('--vaadin-app-layout-touch-optimized', '--vaadin-app-layout-drawer-overlay');
207+
183208
this.$.drawer.addEventListener('transitionstart', () => {
184209
this.__isDrawerAnimating = true;
185210
});
@@ -328,8 +353,6 @@ export const AppLayoutMixin = (superclass) =>
328353
/** @private */
329354
_resize() {
330355
this._blockAnimationUntilAfterNextRender();
331-
this._updateTouchOptimizedMode();
332-
this._updateOverlayMode();
333356
}
334357

335358
/** @protected */

‎packages/app-layout/src/vaadin-app-layout.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class AppLayout extends AppLayoutMixin(ElementMixin(ThemableMixin(PolylitMixin(L
144144
<div hidden>
145145
<slot id="touchSlot" name="navbar touch-optimized" @slotchange="${this.__onNavbarSlotChange}"></slot>
146146
</div>
147+
<div id="cssPropertyObserver"></div>
147148
`;
148149
}
149150
}

‎packages/app-layout/test/app-layout.test.js‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,6 @@ describe('vaadin-app-layout', () => {
131131
expect(layout.$.navbarBottom.hasAttribute('hidden')).to.be.true;
132132
});
133133

134-
it('should remove hidden attribute on non-empty navbar-bottom on resize', () => {
135-
const header = document.createElement('h1');
136-
header.textContent = 'Header';
137-
header.setAttribute('slot', 'navbar touch-optimized');
138-
layout.appendChild(header);
139-
expect(layout.$.navbarBottom.hasAttribute('hidden')).to.be.true;
140-
window.dispatchEvent(new Event('resize'));
141-
expect(layout.$.navbarBottom.hasAttribute('hidden')).to.be.false;
142-
});
143-
144134
it('should update content offset when navbar height changes', async () => {
145135
// Add content to navbar and measure original offset
146136
const navbarContent = document.createElement('div');
@@ -163,7 +153,7 @@ describe('vaadin-app-layout', () => {
163153
let drawer, backdrop, toggle;
164154

165155
async function fixtureLayout(layoutMode) {
166-
const overlayMode = String(layoutMode === 'mobile');
156+
const overlayMode = layoutMode === 'mobile' ? 'true' : 'false';
167157

168158
layout = fixtureSync(`
169159
<vaadin-app-layout style="--vaadin-app-layout-drawer-overlay: ${overlayMode}; --vaadin-app-layout-transition-duration: 0s;">

‎packages/app-layout/test/visual/base/app-layout.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ describe('app-layout', () => {
4444

4545
it('overlay', async () => {
4646
// See https://github.com/vaadin/vaadin-app-layout/issues/183
47-
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' true');
47+
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' 1');
4848
window.dispatchEvent(new Event('resize'));
4949
await visualDiff(div, `${dir}-overlay`);
5050
});
5151

5252
it('primary-drawer-overlay-opened', async () => {
5353
element.primarySection = 'drawer';
54-
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' true');
54+
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' 1');
5555
window.dispatchEvent(new Event('resize'));
5656
element.drawerOpened = true;
5757
await visualDiff(div, `${dir}-primary-drawer-overlay-opened`);

‎packages/app-layout/test/visual/lumo/app-layout.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,14 @@ describe('app-layout', () => {
4747

4848
it('overlay', async () => {
4949
// See https://github.com/vaadin/vaadin-app-layout/issues/183
50-
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' true');
50+
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' 1');
5151
window.dispatchEvent(new Event('resize'));
5252
await visualDiff(div, `${dir}-overlay`);
5353
});
5454

5555
it('primary-drawer-overlay-opened', async () => {
5656
element.primarySection = 'drawer';
57-
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' true');
57+
element.style.setProperty('--vaadin-app-layout-drawer-overlay', ' 1');
5858
window.dispatchEvent(new Event('resize'));
5959
element.drawerOpened = true;
6060
await visualDiff(div, `${dir}-primary-drawer-overlay-opened`);

‎packages/component-base/src/css-property-observer.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* @license
3+
* Copyright (c) 2000 - 2025 Vaadin Ltd.
4+
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
5+
*/
6+
7+
/**
8+
* WARNING: For internal use only. Do not use this class in custom components.
9+
*
10+
* @private
11+
*/
12+
export class CSSPropertyObserver extends EventTarget {
13+
element;
14+
callback;
15+
properties = new Set();
16+
17+
constructor(element, callback) {
18+
super();
19+
this.element = element;
20+
this._handleTransitionEvent = this._handleTransitionEvent.bind(this);
21+
22+
if (callback) {
23+
this.addEventListener('property-changed', (event) => callback(event.detail.propertyName));
24+
}
25+
}
26+
27+
observe(...properties) {
28+
this.connect();
29+
30+
const newProperties = properties.filter((property) => !this.properties.has(property));
31+
if (newProperties.length > 0) {
32+
newProperties.forEach((property) => this.properties.add(property));
33+
this._updateStyles();
34+
}
35+
}
36+
37+
connect() {
38+
this.element.addEventListener('transitionend', this._handleTransitionEvent);
39+
}
40+
41+
disconnect() {
42+
this.properties.clear();
43+
this.element.removeEventListener('transitionend', this._handleTransitionEvent);
44+
}
45+
46+
/** @protected */
47+
_handleTransitionEvent(event) {
48+
const { propertyName } = event;
49+
this.dispatchEvent(new CustomEvent('property-changed', { detail: { propertyName } }));
50+
}
51+
52+
/** @protected */
53+
_updateStyles() {
54+
this.element.style.display = 'contents';
55+
this.element.style.transitionDuration = '1ms';
56+
this.element.style.transitionBehavior = 'allow-discrete';
57+
this.element.style.transitionProperty = `${[...this.properties].join(', ')}`;
58+
this.element.style.transitionTimingFunction = 'step-end';
59+
}
60+
}

‎packages/component-base/src/styles/style-props.js‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,23 @@ import { addGlobalThemeStyles } from '@vaadin/vaadin-themable-mixin/register-sty
1010
// here to avoid performance issues in Aura. Aura overrides these properties with
1111
// values that use complex, nested relative color functions, and without explicit
1212
// typing, Chrome 142 was found to render components that use them roughly 40% slower.
13-
[
14-
'--vaadin-text-color',
15-
'--vaadin-text-color-disabled',
16-
'--vaadin-text-color-secondary',
17-
'--vaadin-border-color',
18-
'--vaadin-border-color-secondary',
19-
'--vaadin-background-color',
20-
].forEach((propertyName) => {
21-
CSS.registerProperty({
22-
name: propertyName,
23-
syntax: '<color>',
24-
inherits: true,
25-
// Use this initial value so the color stays visible when the property
26-
// is set to an invalid value to make debugging a bit easier.
27-
initialValue: 'light-dark(black, white)',
28-
});
29-
});
13+
// [
14+
// '--vaadin-text-color',
15+
// '--vaadin-text-color-disabled',
16+
// '--vaadin-text-color-secondary',
17+
// '--vaadin-border-color',
18+
// '--vaadin-border-color-secondary',
19+
// '--vaadin-background-color',
20+
// ].forEach((propertyName) => {
21+
// CSS.registerProperty({
22+
// name: propertyName,
23+
// syntax: '<color>',
24+
// inherits: true,
25+
// // Use this initial value so the color stays visible when the property
26+
// // is set to an invalid value to make debugging a bit easier.
27+
// initialValue: 'light-dark(black, white)',
28+
// });
29+
// });
3030

3131
addGlobalThemeStyles(
3232
'vaadin-base',

‎packages/component-base/test/css-property-observer.test.js‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { expect } from '@vaadin/chai-plugins';
2+
import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
3+
import sinon from 'sinon';
4+
import { CSSPropertyObserver } from '../src/css-property-observer.js';
5+
6+
CSS.registerProperty({
7+
name: '--test-prop-0',
8+
syntax: '<number>',
9+
inherits: true,
10+
initialValue: '0',
11+
});
12+
13+
CSS.registerProperty({
14+
name: '--test-prop-1',
15+
syntax: '<number>',
16+
inherits: true,
17+
initialValue: '0',
18+
});
19+
20+
describe('CSSPropertyObserver', () => {
21+
let observer, element, callback;
22+
23+
beforeEach(async () => {
24+
element = fixtureSync('<div></div>');
25+
callback = sinon.spy();
26+
observer = new CSSPropertyObserver(element, callback);
27+
await nextFrame();
28+
});
29+
30+
it('should observe CSS property changes', async () => {
31+
observer.observe('--test-prop-0', '--test-prop-1');
32+
await nextFrame();
33+
expect(callback).to.be.not.called;
34+
35+
element.style.setProperty('--test-prop-0', '1');
36+
await nextFrame();
37+
expect(callback).to.be.calledOnceWith('--test-prop-0');
38+
39+
callback.resetHistory();
40+
41+
element.style.setProperty('--test-prop-1', '1');
42+
await nextFrame();
43+
expect(callback).to.be.calledOnceWith('--test-prop-1');
44+
});
45+
46+
it('should stop observing when disconnect is called', async () => {
47+
observer.observe('--test-prop-0');
48+
await nextFrame();
49+
observer.disconnect();
50+
await nextFrame();
51+
52+
element.style.setProperty('--test-prop-0', '1');
53+
await nextFrame();
54+
expect(callback).to.be.not.called;
55+
});
56+
});

‎packages/vaadin-themable-mixin/src/css-property-observer.js‎

Lines changed: 0 additions & 90 deletions
This file was deleted.

‎packages/vaadin-themable-mixin/src/lumo-injector.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
* Copyright (c) 2021 - 2025 Vaadin Ltd.
44
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
55
*/
6-
import { CSSPropertyObserver } from './css-property-observer.js';
76
import { injectLumoStyleSheet, removeLumoStyleSheet } from './css-utils.js';
87
import { parseStyleSheets } from './lumo-modules.js';
8+
import { RootCSSPropertyObserver } from './root-css-property-observer.js';
99

1010
export function getLumoInjectorPropName(lumoInjector) {
1111
return `--_lumo-${lumoInjector.is}-inject`;
@@ -87,7 +87,7 @@ export class LumoInjector {
8787
constructor(root = document) {
8888
this.#root = root;
8989
this.handlePropertyChange = this.handlePropertyChange.bind(this);
90-
this.#cssPropertyObserver = CSSPropertyObserver.for(root);
90+
this.#cssPropertyObserver = RootCSSPropertyObserver.for(root);
9191
this.#cssPropertyObserver.addEventListener('property-changed', this.handlePropertyChange);
9292
}
9393

0 commit comments

Comments
 (0)