Merge pull request #1331 from ing-bank/fix/focusElementAfterHideWhenOwned

fix(overlays): only focus controlled elements after hide
This commit is contained in:
Thijs Louisse 2021-04-13 12:09:06 +02:00 committed by GitHub
commit a94fa77b62
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 39 additions and 27 deletions

View file

@ -0,0 +1,5 @@
---
'@lion/checkbox-group': patch
---
recover possibility to registrer without []

View file

@ -0,0 +1,5 @@
---
'@lion/overlays': patch
---
only focus elements after hide when OverlayController has ownership

View file

@ -9,12 +9,4 @@ export class LionCheckboxGroup extends ChoiceGroupMixin(FormGroupMixin(LitElemen
super(); super();
this.multipleChoice = true; this.multipleChoice = true;
} }
/** @param {import('@lion/core').PropertyValues } changedProperties */
updated(changedProperties) {
super.updated(changedProperties);
if (changedProperties.has('name') && !String(this.name).match(/\[\]$/)) {
throw new Error('Names should end in "[]".');
}
}
} }

View file

@ -104,17 +104,4 @@ describe('<lion-checkbox-group>', () => {
`); `);
await expect(el).to.be.accessible(); await expect(el).to.be.accessible();
}); });
it("should throw exception if name doesn't end in []", async () => {
const el = await fixture(html`<lion-checkbox-group name="woof[]"></lion-checkbox-group>`);
el.name = 'woof';
let err;
try {
await el.updateComplete;
} catch (e) {
err = e;
}
expect(err).to.be.an.instanceof(Error);
expect(err.message).to.equal('Names should end in "[]".');
});
}); });

View file

@ -907,15 +907,19 @@ export class OverlayController extends EventTargetShim {
/** @protected */ /** @protected */
_restoreFocus() { _restoreFocus() {
const { activeElement } = /** @type {* & ShadowRoot} */ (this
.__contentWrapperNode).getRootNode();
// We only are allowed to move focus if we (still) 'own' it. // We only are allowed to move focus if we (still) 'own' it.
// Otherwise we assume the 'outside world' has, purposefully, taken over // Otherwise we assume the 'outside world' has, purposefully, taken over
if (
activeElement &&
/** @type {HTMLElement} */ (this.__contentWrapperNode).contains(activeElement)
) {
if (this.elementToFocusAfterHide) { if (this.elementToFocusAfterHide) {
this.elementToFocusAfterHide.focus(); this.elementToFocusAfterHide.focus();
} else if ( } else {
document.activeElement && activeElement.blur();
this.__contentWrapperNode?.contains(document.activeElement) }
) {
/** @type {HTMLElement} */ (document.activeElement).blur();
} }
} }

View file

@ -741,6 +741,25 @@ describe('OverlayController', () => {
expect(document.activeElement).to.equal(input); expect(document.activeElement).to.equal(input);
}); });
it(`only sets focus when outside world didn't take over already`, async () => {
const input = /** @type {HTMLElement} */ (await fixture('<input />'));
const outsideButton = /** @type {HTMLButtonElement} */ (await fixture('<button></button>'));
const contentNode = /** @type {HTMLElement} */ (await fixture('<div>/div>'));
const ctrl = new OverlayController({
...withGlobalTestConfig(),
elementToFocusAfterHide: input,
contentNode,
});
await ctrl.show();
// an outside element has taken over focus
outsideButton.focus();
expect(document.activeElement).to.equal(outsideButton);
await ctrl.hide();
expect(document.activeElement).to.equal(outsideButton);
});
it('allows to set elementToFocusAfterHide on show', async () => { it('allows to set elementToFocusAfterHide on show', async () => {
const input = /** @type {HTMLElement} */ (await fixture('<input />')); const input = /** @type {HTMLElement} */ (await fixture('<input />'));
const contentNode = /** @type {HTMLElement} */ (await fixture( const contentNode = /** @type {HTMLElement} */ (await fixture(