ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(button): make click event target predictable (always host)

Browse source
This commit is contained in:
Thijs Louisse 2020-11-11 15:05:44 +01:00
parent 84ec97a640
commit 27020f12af
3 changed files with 136 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
.changeset/afraid-maps-fix.md Normal file
View file

@ -0,0 +1,8 @@
---
'@lion/button': patch
---
Button fixes
- make click event target predictable (always host)
- do not override aria-labelledby from user

58
packages/button/src/LionButton.js
View file

@ -146,10 +146,6 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
));
}
get _form() {
return this._nativeButtonNode.form;
}
// @ts-ignore
get slots() {
return {
@ -179,16 +175,24 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
}
});
}
/** @type {HTMLButtonElement} */
this.__submitAndResetHelperButton = document.createElement('button');
/** @type {EventListener} */
this.__preventEventLeakage = this.__preventEventLeakage.bind(this);
}
connectedCallback() {
super.connectedCallback();
this.__setupEvents();
this.__setupSubmitAndResetHelperOnConnected();
}
disconnectedCallback() {
super.disconnectedCallback();
this.__teardownEvents();
this.__teardownSubmitAndResetHelperOnDisconnected();
}
/**
@ -211,12 +215,24 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
* Delegate click, by flashing a native button as a direct child
* of the form, and firing click on this button. This will fire the form submit
* without side effects caused by the click bubbling back up to lion-button.
* @param {Event} e
* @param {Event} ev
*/
__clickDelegationHandler(e) {
if ((this.type === 'submit' || this.type === 'reset') && e.target === this && this._form) {
e.stopImmediatePropagation();
this._nativeButtonNode.click();
__clickDelegationHandler(ev) {
if ((this.type === 'submit' || this.type === 'reset') && ev.target === this && this._form) {
/**
* Here, we make sure our button is compatible with a native form, by firing a click
* from a native button that our form responds to. The native button we spawn will be a direct
* child of the form, plus the click event that will be sent will be prevented from
* propagating outside of the form. This will keep the amount of 'noise' (click events
* from 'ghost elements' that can be intercepted by listeners in the bubble chain) to an
* absolute minimum.
*/
this.__submitAndResetHelperButton.type = this.type;
this._form.appendChild(this.__submitAndResetHelperButton);
// Form submission or reset will happen
this.__submitAndResetHelperButton.click();
this._form.removeChild(this.__submitAndResetHelperButton);
}
}
@ -291,4 +307,28 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
this.click();
}
}
/**
* Prevents that someone who listens outside or on form catches the click event
* @param {Event} e
*/
__preventEventLeakage(e) {
if (e.target === this.__submitAndResetHelperButton) {
e.stopImmediatePropagation();
}
}
__setupSubmitAndResetHelperOnConnected() {
this._form = this._nativeButtonNode.form;
if (this._form) {
this._form.addEventListener('click', this.__preventEventLeakage);
}
}
__teardownSubmitAndResetHelperOnDisconnected() {
if (this._form) {
this._form.removeEventListener('click', this.__preventEventLeakage);
}
}
}

105
packages/button/test/lion-button.test.js
View file

@ -359,9 +359,8 @@ describe('lion-button', () => {
</form>
`);
/** @type {LionButton} */ form
.querySelector('lion-button')
.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' }));
const lionButton = /** @type {LionButton} */ (form.querySelector('lion-button'));
lionButton.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' }));
await aTimeout(0);
await aTimeout(0);
@ -431,24 +430,82 @@ describe('lion-button', () => {
expect(clickSpy).to.have.been.calledOnce;
});
it('is fired one inside a form', async () => {
const formClickSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const el = /** @type {HTMLFormElement} */ (await fixture(
it('is fired once outside and inside the form', async () => {
const outsideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const insideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const formSpyEarly = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const formSpyLater = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const el = /** @type {HTMLDivElement} */ (await fixture(
html`
<form @click="${formClickSpy}">
<lion-button>foo</lion-button>
</form>
<div @click="${outsideSpy}">
<form @click="${formSpyEarly}">
<div @click="${insideSpy}">
<lion-button>foo</lion-button>
</div>
</form>
</div>
`,
));
const lionButton = /** @type {LionButton} */ (el.querySelector('lion-button'));
const form = /** @type {HTMLFormElement} */ (el.querySelector('form'));
form.addEventListener('click', formSpyLater);
// @ts-ignore
el.querySelector('lion-button').click();
lionButton.click();
// trying to wait for other possible redispatched events
await aTimeout(0);
await aTimeout(0);
expect(formClickSpy).to.have.been.calledOnce;
expect(insideSpy).to.have.been.calledOnce;
expect(outsideSpy).to.have.been.calledOnce;
// A small sacrifice for event listeners registered early: we get the native button evt.
expect(formSpyEarly).to.have.been.calledTwice;
expect(formSpyLater).to.have.been.calledOnce;
});
it('works when connected to different form', async () => {
const form1El = /** @type {HTMLFormElement} */ (await fixture(
html`
<form>
<lion-button>foo</lion-button>
</form>
`,
));
const lionButton = /** @type {LionButton} */ (form1El.querySelector('lion-button'));
expect(lionButton._form).to.equal(form1El);
// Now we add the lionButton to a different form.
// We disconnect and connect and check if everything still works as expected
const outsideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const insideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const formSpyEarly = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const formSpyLater = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const form2El = /** @type {HTMLFormElement} */ (await fixture(
html`
<div @click="${outsideSpy}">
<form @click="${formSpyEarly}">
<div @click="${insideSpy}">${lionButton}</div>
</form>
</div>
`,
));
const form2Node = /** @type {HTMLFormElement} */ (form2El.querySelector('form'));
expect(lionButton._form).to.equal(form2Node);
form2Node.addEventListener('click', formSpyLater);
lionButton.click();
// trying to wait for other possible redispatched events
await aTimeout(0);
await aTimeout(0);
expect(insideSpy).to.have.been.calledOnce;
expect(outsideSpy).to.have.been.calledOnce;
// A small sacrifice for event listeners registered early: we get the native button evt.
expect(formSpyEarly).to.have.been.calledTwice;
expect(formSpyLater).to.have.been.calledOnce;
});
describe('native button behavior', async () => {
@ -490,17 +547,17 @@ describe('lion-button', () => {
});
const useCases = [
{ container: 'div', type: 'submit', targetHost: true },
{ container: 'div', type: 'reset', targetHost: true },
{ container: 'div', type: 'button', targetHost: true },
{ container: 'form', type: 'submit', targetHost: false },
{ container: 'form', type: 'reset', targetHost: false },
{ container: 'form', type: 'button', targetHost: true },
{ container: 'div', type: 'submit' },
{ container: 'div', type: 'reset' },
{ container: 'div', type: 'button' },
{ container: 'form', type: 'submit' },
{ container: 'form', type: 'reset' },
{ container: 'form', type: 'button' },
];
useCases.forEach(useCase => {
const { container, type, targetHost } = useCase;
const targetName = targetHost ? 'host' : 'native button';
const { container, type } = useCase;
const targetName = 'host';
it(`is ${targetName} with type ${type} and it is inside a ${container}`, async () => {
const clickSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
const el = /** @type {LionButton} */ (await fixture(
@ -510,11 +567,7 @@ describe('lion-button', () => {
await fixture(html`<${tag} @click="${clickSpy}">${el}</${tag}>`);
const event = await prepareClickEvent(el);
if (targetHost) {
expect(event.target).to.equal(el);
} else {
expect(event.target).to.equal(el._nativeButtonNode);
}
expect(event.target).to.equal(el);
});
});
});