Skip to content

Commit df8acb8

Browse files
fix: prevent scroll when restoring focus not from keyboard (#7599) (#7603)
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
1 parent 3fb9d66 commit df8acb8

File tree

4 files changed

+77
-6
lines changed

4 files changed

+77
-6
lines changed

packages/a11y-base/src/focus-restoration-controller.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,22 @@ export class FocusRestorationController {
2323
/**
2424
* Restores focus to the target node that was saved previously with `saveFocus()`.
2525
*/
26-
restoreFocus() {
26+
restoreFocus(options) {
2727
const focusNode = this.focusNode;
2828
if (!focusNode) {
2929
return;
3030
}
3131

32+
const preventScroll = options ? options.preventScroll : false;
33+
3234
if (getDeepActiveElement() === document.body) {
3335
// In Firefox and Safari, focusing the node synchronously
3436
// doesn't work as expected when the overlay is closing on outside click.
3537
// These browsers force focus to move to the body element and retain it
3638
// there until the next event loop iteration.
37-
setTimeout(() => focusNode.focus());
39+
setTimeout(() => focusNode.focus({ preventScroll }));
3840
} else {
39-
focusNode.focus();
41+
focusNode.focus({ preventScroll });
4042
}
4143

4244
this.focusNode = null;

packages/a11y-base/test/focus-restoration-controller.test.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect } from '@esm-bundle/chai';
22
import { aTimeout, fixtureSync, outsideClick } from '@vaadin/testing-helpers';
3+
import sinon from 'sinon';
34
import { FocusRestorationController } from '../src/focus-restoration-controller.js';
45
import { getDeepActiveElement } from '../src/focus-utils.js';
56

@@ -58,4 +59,44 @@ describe('focus-restoration-controller', () => {
5859
controller.restoreFocus();
5960
expect(getDeepActiveElement()).to.equal(button2);
6061
});
62+
63+
it('should not prevent scroll when restoring focus synchronously by default', () => {
64+
button1.focus();
65+
const spy = sinon.spy(button2, 'focus');
66+
controller.saveFocus(button2);
67+
controller.restoreFocus();
68+
expect(spy).to.be.calledOnce;
69+
expect(spy.firstCall.args[0]).to.eql({ preventScroll: false });
70+
});
71+
72+
it('should prevent scroll when restoring focus synchronously with preventScroll', () => {
73+
button1.focus();
74+
const spy = sinon.spy(button2, 'focus');
75+
controller.saveFocus(button2);
76+
controller.restoreFocus({ preventScroll: true });
77+
expect(spy).to.be.calledOnce;
78+
expect(spy.firstCall.args[0]).to.eql({ preventScroll: true });
79+
});
80+
81+
it('should not prevent scroll when restoring focus asynchronously by default', async () => {
82+
button1.focus();
83+
const spy = sinon.spy(button2, 'focus');
84+
controller.saveFocus(button2);
85+
outsideClick();
86+
controller.restoreFocus();
87+
await aTimeout(0);
88+
expect(spy).to.be.calledOnce;
89+
expect(spy.firstCall.args[0]).to.eql({ preventScroll: false });
90+
});
91+
92+
it('should prevent scroll when restoring focus asynchronously with preventScroll', async () => {
93+
button1.focus();
94+
const spy = sinon.spy(button2, 'focus');
95+
controller.saveFocus(button2);
96+
outsideClick();
97+
controller.restoreFocus({ preventScroll: true });
98+
await aTimeout(0);
99+
expect(spy).to.be.calledOnce;
100+
expect(spy.firstCall.args[0]).to.eql({ preventScroll: true });
101+
});
61102
});

packages/overlay/src/vaadin-overlay-focus-mixin.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { AriaModalController } from '@vaadin/a11y-base/src/aria-modal-controller.js';
77
import { FocusRestorationController } from '@vaadin/a11y-base/src/focus-restoration-controller.js';
88
import { FocusTrapController } from '@vaadin/a11y-base/src/focus-trap-controller.js';
9-
import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
9+
import { getDeepActiveElement, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
1010
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
1111

1212
/**
@@ -76,7 +76,8 @@ export const OverlayFocusMixin = (superClass) =>
7676
}
7777

7878
if (this.restoreFocusOnClose && this._shouldRestoreFocus()) {
79-
this.__focusRestorationController.restoreFocus();
79+
const preventScroll = !isKeyboardActive();
80+
this.__focusRestorationController.restoreFocus({ preventScroll });
8081
}
8182
}
8283

packages/overlay/test/restore-focus.common.js

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect } from '@esm-bundle/chai';
2-
import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
2+
import { escKeyDown, fixtureSync, mousedown, nextRender } from '@vaadin/testing-helpers';
3+
import sinon from 'sinon';
34
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
45
import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
56

@@ -142,6 +143,32 @@ describe('restore focus', () => {
142143
expect(getDeepActiveElement()).to.equal(focusInput);
143144
});
144145
});
146+
147+
describe('prevent scroll', () => {
148+
it('should prevent scroll when restoring focus on close after mousedown', async () => {
149+
focusable.focus();
150+
overlay.opened = true;
151+
await nextRender();
152+
const spy = sinon.spy(focusable, 'focus');
153+
mousedown(document.body);
154+
overlay.opened = false;
155+
await nextRender();
156+
expect(spy).to.be.calledOnce;
157+
expect(spy.firstCall.args[0]).to.eql({ preventScroll: true });
158+
});
159+
160+
it('should not prevent scroll when restoring focus on close after keydown', async () => {
161+
focusable.focus();
162+
overlay.opened = true;
163+
await nextRender();
164+
const spy = sinon.spy(focusable, 'focus');
165+
escKeyDown(document.body);
166+
overlay.opened = false;
167+
await nextRender();
168+
expect(spy).to.be.calledOnce;
169+
expect(spy.firstCall.args[0]).to.eql({ preventScroll: false });
170+
});
171+
});
145172
});
146173
});
147174
});

0 commit comments

Comments
 (0)