Skip to content

Commit 40e424a

Browse files
fix: avoid scrolling when dialog is focused (#10686) (#10689)
* fix: avoid scrolling when dialog is focused * fix popover * cleanup tests * fix login overlay * fix crud and rte dialogs Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
1 parent 620b1cd commit 40e424a

File tree

10 files changed

+93
-5
lines changed

10 files changed

+93
-5
lines changed

packages/confirm-dialog/src/vaadin-confirm-dialog.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class ConfirmDialog extends ConfirmDialogMixin(ElementMixin(ThemePropertyMixin(P
9494
:host([opening]),
9595
:host([closing]) {
9696
display: block !important;
97-
position: absolute;
97+
position: fixed;
9898
outline: none;
9999
}
100100

packages/confirm-dialog/test/confirm-dialog.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,24 @@ describe('vaadin-confirm-dialog', () => {
712712
await aTimeout(0);
713713
expect(getDeepActiveElement()).to.equal(button);
714714
});
715+
716+
it('should not scroll the page when opening the dialog', async () => {
717+
// Create a tall element to make the page scrollable
718+
const spacer = fixtureSync(`
719+
<div style="height: 200vh"></div>
720+
`);
721+
document.body.insertBefore(spacer, document.body.firstChild);
722+
723+
// Scroll to the top
724+
window.scrollTo(0, 0);
725+
726+
// Open the dialog (which will focus it)
727+
confirm.opened = true;
728+
await oneEvent(overlay, 'vaadin-overlay-open');
729+
730+
// The page should not have scrolled
731+
expect(window.scrollY).to.equal(0);
732+
});
715733
});
716734

717735
describe('detach and re-attach', () => {

packages/crud/src/vaadin-crud-dialog.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class CrudDialog extends DialogBaseMixin(ThemePropertyMixin(PolylitMixin(LitElem
101101
:host([opening]),
102102
:host([closing]) {
103103
display: block !important;
104-
position: absolute;
104+
position: fixed;
105105
}
106106
107107
:host,

packages/dialog/src/vaadin-dialog.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class Dialog extends DialogSizeMixin(
120120
:host([opening]),
121121
:host([closing]) {
122122
display: block !important;
123-
position: absolute;
123+
position: fixed;
124124
outline: none;
125125
}
126126

packages/dialog/test/dialog.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,24 @@ describe('vaadin-dialog', () => {
276276
await nextRender();
277277
expect(getDeepActiveElement()).to.equal(button);
278278
});
279+
280+
it('should not scroll the page when opening the dialog', async () => {
281+
// Create a tall element to make the page scrollable
282+
const spacer = fixtureSync(`
283+
<div style="height: 200vh"></div>
284+
`);
285+
document.body.insertBefore(spacer, document.body.firstChild);
286+
287+
// Scroll to the top
288+
window.scrollTo(0, 0);
289+
290+
// Open the dialog (which will focus it)
291+
dialog.opened = true;
292+
await oneEvent(overlay, 'vaadin-overlay-open');
293+
294+
// The page should not have scrolled
295+
expect(window.scrollY).to.equal(0);
296+
});
279297
});
280298

281299
describe('position/sizing', () => {

packages/login/src/vaadin-login-overlay.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class LoginOverlay extends LoginFormMixin(LoginOverlayMixin(ElementMixin(Themabl
7272
:host([opening]),
7373
:host([closing]) {
7474
display: block !important;
75-
position: absolute;
75+
position: fixed;
7676
outline: none;
7777
}
7878

packages/login/test/login-overlay.test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,34 @@ describe('no autofocus', () => {
132132
});
133133
});
134134

135+
describe('scroll behavior', () => {
136+
let login, overlay;
137+
138+
beforeEach(async () => {
139+
login = fixtureSync('<vaadin-login-overlay></vaadin-login-overlay>');
140+
await nextRender();
141+
overlay = login.$.overlay;
142+
});
143+
144+
it('should not scroll the page when opening the overlay', async () => {
145+
// Create a tall element to make the page scrollable
146+
const spacer = fixtureSync(`
147+
<div style="height: 200vh"></div>
148+
`);
149+
document.body.insertBefore(spacer, document.body.firstChild);
150+
151+
// Scroll to the top
152+
window.scrollTo(0, 0);
153+
154+
// Open the overlay (which will focus the username field)
155+
login.opened = true;
156+
await oneEvent(overlay, 'vaadin-overlay-open');
157+
158+
// The page should not have scrolled
159+
expect(window.scrollY).to.equal(0);
160+
});
161+
});
162+
135163
describe('title and description', () => {
136164
let login, overlay, titleElement, descriptionElement;
137165

packages/popover/src/vaadin-popover.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ class Popover extends PopoverPositionMixin(
229229
:host([opening]),
230230
:host([closing]) {
231231
display: block !important;
232-
position: absolute;
232+
position: fixed;
233233
outline: none;
234234
}
235235

packages/popover/test/a11y.test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,29 @@ describe('a11y', () => {
398398
expect(focusSpy).to.be.calledOnce;
399399
});
400400
});
401+
402+
describe('scroll behavior', () => {
403+
it('should not scroll the page when focusing the popover', async () => {
404+
// Create a tall element to make the page scrollable
405+
const spacer = fixtureSync(`
406+
<div style="height: 200vh"></div>
407+
`);
408+
document.body.insertBefore(spacer, document.body.firstChild);
409+
410+
// Scroll to the top
411+
window.scrollTo(0, 0);
412+
413+
// Open the popover
414+
popover.opened = true;
415+
await oneEvent(overlay, 'vaadin-overlay-open');
416+
417+
// Focus the popover programmatically
418+
popover.focus();
419+
420+
// The page should not have scrolled
421+
expect(window.scrollY).to.equal(0);
422+
});
423+
});
401424
});
402425

403426
describe('Tab order', () => {

packages/rich-text-editor/src/vaadin-rich-text-editor-popup.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class RichTextEditorPopup extends PolylitMixin(LitElement) {
3636
:host([opening]),
3737
:host([closing]) {
3838
display: contents !important;
39+
position: fixed;
3940
}
4041
4142
:host,

0 commit comments

Comments
 (0)