Skip to content

Commit

Permalink
fix: prevent scroll when restoring focus not from keyboard (#7599) (#…
Browse files Browse the repository at this point in the history
…7602)

Co-authored-by: Serhii Kulykov <[email protected]>
  • Loading branch information
vaadin-bot and web-padawan committed Jul 30, 2024
1 parent e72b800 commit 124e60d
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 6 deletions.
8 changes: 5 additions & 3 deletions packages/a11y-base/src/focus-restoration-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,22 @@ export class FocusRestorationController {
/**
* Restores focus to the target node that was saved previously with `saveFocus()`.
*/
restoreFocus() {
restoreFocus(options) {
const focusNode = this.focusNode;
if (!focusNode) {
return;
}

const preventScroll = options ? options.preventScroll : false;

if (getDeepActiveElement() === document.body) {
// In Firefox and Safari, focusing the node synchronously
// doesn't work as expected when the overlay is closing on outside click.
// These browsers force focus to move to the body element and retain it
// there until the next event loop iteration.
setTimeout(() => focusNode.focus());
setTimeout(() => focusNode.focus({ preventScroll }));
} else {
focusNode.focus();
focusNode.focus({ preventScroll });
}

this.focusNode = null;
Expand Down
41 changes: 41 additions & 0 deletions packages/a11y-base/test/focus-restoration-controller.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@esm-bundle/chai';
import { aTimeout, fixtureSync, outsideClick } from '@vaadin/testing-helpers';
import sinon from 'sinon';
import { FocusRestorationController } from '../src/focus-restoration-controller.js';
import { getDeepActiveElement } from '../src/focus-utils.js';

Expand Down Expand Up @@ -58,4 +59,44 @@ describe('focus-restoration-controller', () => {
controller.restoreFocus();
expect(getDeepActiveElement()).to.equal(button2);
});

it('should not prevent scroll when restoring focus synchronously by default', () => {
button1.focus();
const spy = sinon.spy(button2, 'focus');
controller.saveFocus(button2);
controller.restoreFocus();
expect(spy).to.be.calledOnce;
expect(spy.firstCall.args[0]).to.eql({ preventScroll: false });
});

it('should prevent scroll when restoring focus synchronously with preventScroll', () => {
button1.focus();
const spy = sinon.spy(button2, 'focus');
controller.saveFocus(button2);
controller.restoreFocus({ preventScroll: true });
expect(spy).to.be.calledOnce;
expect(spy.firstCall.args[0]).to.eql({ preventScroll: true });
});

it('should not prevent scroll when restoring focus asynchronously by default', async () => {
button1.focus();
const spy = sinon.spy(button2, 'focus');
controller.saveFocus(button2);
outsideClick();
controller.restoreFocus();
await aTimeout(0);
expect(spy).to.be.calledOnce;
expect(spy.firstCall.args[0]).to.eql({ preventScroll: false });
});

it('should prevent scroll when restoring focus asynchronously with preventScroll', async () => {
button1.focus();
const spy = sinon.spy(button2, 'focus');
controller.saveFocus(button2);
outsideClick();
controller.restoreFocus({ preventScroll: true });
await aTimeout(0);
expect(spy).to.be.calledOnce;
expect(spy.firstCall.args[0]).to.eql({ preventScroll: true });
});
});
5 changes: 3 additions & 2 deletions packages/overlay/src/vaadin-overlay-focus-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { AriaModalController } from '@vaadin/a11y-base/src/aria-modal-controller.js';
import { FocusRestorationController } from '@vaadin/a11y-base/src/focus-restoration-controller.js';
import { FocusTrapController } from '@vaadin/a11y-base/src/focus-trap-controller.js';
import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
import { getDeepActiveElement, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';

/**
Expand Down Expand Up @@ -76,7 +76,8 @@ export const OverlayFocusMixin = (superClass) =>
}

if (this.restoreFocusOnClose && this._shouldRestoreFocus()) {
this.__focusRestorationController.restoreFocus();
const preventScroll = !isKeyboardActive();
this.__focusRestorationController.restoreFocus({ preventScroll });
}
}

Expand Down
29 changes: 28 additions & 1 deletion packages/overlay/test/restore-focus.common.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@esm-bundle/chai';
import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
import { escKeyDown, fixtureSync, mousedown, nextRender } from '@vaadin/testing-helpers';
import sinon from 'sinon';
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';

Expand Down Expand Up @@ -142,6 +143,32 @@ describe('restore focus', () => {
expect(getDeepActiveElement()).to.equal(focusInput);
});
});

describe('prevent scroll', () => {
it('should prevent scroll when restoring focus on close after mousedown', async () => {
focusable.focus();
overlay.opened = true;
await nextRender();
const spy = sinon.spy(focusable, 'focus');
mousedown(document.body);
overlay.opened = false;
await nextRender();
expect(spy).to.be.calledOnce;
expect(spy.firstCall.args[0]).to.eql({ preventScroll: true });
});

it('should not prevent scroll when restoring focus on close after keydown', async () => {
focusable.focus();
overlay.opened = true;
await nextRender();
const spy = sinon.spy(focusable, 'focus');
escKeyDown(document.body);
overlay.opened = false;
await nextRender();
expect(spy).to.be.calledOnce;
expect(spy.firstCall.args[0]).to.eql({ preventScroll: false });
});
});
});
});
});

0 comments on commit 124e60d

Please sign in to comment.