Skip to content

Commit 430bd3a

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

File tree

9 files changed

+206
-107
lines changed

9 files changed

+206
-107
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;">
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+
}
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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @license
3+
* Copyright (c) 2021 - 2025 Vaadin Ltd.
4+
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
5+
*/
6+
import { CSSPropertyObserver } from '@vaadin/component-base/src/css-property-observer.js';
7+
8+
/**
9+
* An extension of CSSPropertyObserver that takes a Document or ShadowRoot and
10+
* listens for changes to CSS custom properties on its `:root` or `:host` element
11+
* via the `::before` pseudo-element.
12+
*
13+
* WARNING: For internal use only. Do not use this class in custom components.
14+
*/
15+
export class RootCSSPropertyObserver extends CSSPropertyObserver {
16+
#root;
17+
#styleSheet = new CSSStyleSheet();
18+
19+
/**
20+
* Gets or creates the CSSPropertyObserver for the given root.
21+
* @param {DocumentOrShadowRoot} root
22+
* @returns {RootCSSPropertyObserver}
23+
*/
24+
static for(root) {
25+
root.__cssPropertyObserver ||= new RootCSSPropertyObserver(root);
26+
return root.__cssPropertyObserver;
27+
}
28+
29+
constructor(root) {
30+
super(root.host ?? root.documentElement);
31+
this.#root = root;
32+
}
33+
34+
connect() {
35+
super.connect();
36+
this.#root.adoptedStyleSheets = this.#root.adoptedStyleSheets.filter((s) => s !== this.#styleSheet);
37+
this.#root.adoptedStyleSheets.unshift(this.#styleSheet);
38+
}
39+
40+
disconnect() {
41+
super.disconnect();
42+
this.#root.adoptedStyleSheets = this.#root.adoptedStyleSheets.filter((s) => s !== this.#styleSheet);
43+
}
44+
45+
/** @override */
46+
_updateStyles() {
47+
this.#styleSheet.replaceSync(`
48+
:root::before, :host::before {
49+
content: '' !important;
50+
position: absolute !important;
51+
top: -9999px !important;
52+
left: -9999px !important;
53+
visibility: hidden !important;
54+
transition: 1ms allow-discrete step-end !important;
55+
transition-property: ${[...this.properties].join(', ')} !important;
56+
}
57+
`);
58+
}
59+
}

packages/vaadin-themable-mixin/src/theme-detector.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Copyright (c) 2000 - 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';
6+
import { RootCSSPropertyObserver } from './root-css-property-observer.js';
77

88
// Register CSS custom properties for observing theme changes
99
CSS.registerProperty({
@@ -43,7 +43,7 @@ export class ThemeDetector extends EventTarget {
4343
this.#root = root;
4444
this.#detectTheme();
4545

46-
this.#observer = CSSPropertyObserver.for(this.#root);
46+
this.#observer = RootCSSPropertyObserver.for(this.#root);
4747
this.#observer.observe('--vaadin-aura-theme');
4848
this.#observer.observe('--vaadin-lumo-theme');
4949
this.#observer.addEventListener('property-changed', this.#boundHandleThemeChange);

0 commit comments

Comments
 (0)