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

fix(overlays): enhanced and documented closeOnOutsideClick

Browse source 
Co-authored-by: Konstantinos Norgias <Konstantinos.Norgias@ing.com>
This commit is contained in:
Thijs Louisse 2021-03-18 11:35:37 +01:00
parent 3aa4783326
commit 412270fa1a
9 changed files with 158 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/forty-taxis-lay.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/overlays': patch
---
fix: only close an overlay if both mousedown and mousep events are outside (for hidesOnOutsideClick)

3
docs/docs/systems/overlays/configuration.md
View file

@ -167,7 +167,8 @@ export const hidesOnOutsideClick = () => {
<demo-overlay-system .config=${hidesOnOutsideClickConfig}>
<button slot="invoker">Click me to open the overlay!</button>
<div slot="content" class="demo-overlay">
Hello! You can close this notification here:
<label for="myInput">Clicking this label should not trigger close</label>
<input id="myInput" />
<button
class="close-button"
@click=${e => e.target.dispatchEvent(new Event('close-overlay', { bubbles: true }))}

5
packages/input-datepicker/test/lion-input-datepicker.test.js
View file

@ -2,6 +2,7 @@ import { LionCalendar, isSameDate } from '@lion/calendar';
import { html, LitElement } from '@lion/core';
import { IsDateDisabled, MaxDate, MinDate, MinMaxDate } from '@lion/form-core';
import { aTimeout, defineCE, expect, fixture as _fixture, nextFrame } from '@open-wc/testing';
import { mimicClick } from '@lion/overlays/test-helpers';
import sinon from 'sinon';
import { setViewport } from '@web/test-runner-commands';
import '@lion/input-datepicker/define';
@ -98,13 +99,13 @@ describe('<lion-input-datepicker>', () => {
expect(elObj.overlayController.isShown).to.equal(false);
});
it('closes the calendar via outside click', async () => {
it('closes the calendar via outside click event', async () => {
const el = await fixture(html`<lion-input-datepicker></lion-input-datepicker>`);
const elObj = new DatepickerInputObject(el);
await elObj.openCalendar();
expect(elObj.overlayController.isShown).to.equal(true);
document.body.click();
mimicClick(document.body);
await aTimeout(0);
expect(elObj.overlayController.isShown).to.be.false;
});

1
packages/overlays/package.json
View file

@ -53,6 +53,7 @@
".": "./index.js",
"./test-suites": "./test-suites/index.js",
"./translations/*": "./translations/*",
"./test-helpers": "./test-helpers.js",
"./docs/": "./docs/"
}
}

85
packages/overlays/src/OverlayController.js
View file

@ -1124,55 +1124,86 @@ export class OverlayController extends EventTargetShim {
const addOrRemoveListener = phase === 'show' ? 'addEventListener' : 'removeEventListener';
if (phase === 'show') {
let wasClickInside = false;
let wasIndirectSynchronousClick = false;
// Handle on capture phase and remember till the next task that there was an inside click
/**
* We listen to click (more specifically mouseup and mousedown) events
* in their capture phase (see our tests about 3rd parties stopping event propagation).
* We define an outside click as follows:
* - both mousedown and mouseup occur outside of content or invoker
*
* This means we have the following flow:
* [1]. (optional) mousedown is triggered on content/invoker
* [2]. mouseup is triggered on document (logic will be scheduled to step 4)
* [3]. (optional) mouseup is triggered on content/invoker
* [4]. mouseup logic is executed on document (its logic is inside a timeout and is thus
* executed after 3)
* [5]. Reset all helper variables that were considered in step [4]
*
*/
/** @type {boolean} */
let wasMouseDownInside = false;
/** @type {boolean} */
let wasMouseUpInside = false;
/** @type {EventListenerOrEventListenerObject} */
this.__preventCloseOutsideClick = () => {
if (wasClickInside) {
// This occurs when a synchronous new click is triggered from a previous click.
// For instance, when we have a label pointing to an input, the platform triggers
// a new click on the input. Not taking this click into account, will hide the overlay
// in `__onCaptureHtmlClick`
wasIndirectSynchronousClick = true;
}
wasClickInside = true;
setTimeout(() => {
wasClickInside = false;
setTimeout(() => {
wasIndirectSynchronousClick = false;
});
});
this.__onInsideMouseDown = () => {
// [1]. was mousedown inside content or invoker
wasMouseDownInside = true;
};
// handle on capture phase and schedule the hide if needed
this.__onInsideMouseUp = () => {
// [3]. was mouseup inside content or invoker
wasMouseUpInside = true;
};
/** @type {EventListenerOrEventListenerObject} */
this.__onCaptureHtmlClick = () => {
this.__onDocumentMouseUp = () => {
// [2]. The captured mouseup goes from top of the document to bottom. We add a timeout,
// so that [3] can be executed before [4]
setTimeout(() => {
if (wasClickInside === false && !wasIndirectSynchronousClick) {
// [4]. Keep open if step 1 (mousedown) or 3 (mouseup) was inside
if (!wasMouseDownInside && !wasMouseUpInside) {
this.hide();
}
// [5]. Reset...
wasMouseDownInside = false;
wasMouseUpInside = false;
});
};
}
this.contentWrapperNode[addOrRemoveListener](
'click',
'mousedown',
/** @type {EventListenerOrEventListenerObject} */
(this.__preventCloseOutsideClick),
(this.__onInsideMouseDown),
true,
);
this.contentWrapperNode[addOrRemoveListener](
'mouseup',
/** @type {EventListenerOrEventListenerObject} */
(this.__onInsideMouseUp),
true,
);
if (this.invokerNode) {
// An invoker click (usually resulting in toggle) should be left to a different part of
// the code
this.invokerNode[addOrRemoveListener](
'click',
'mousedown',
/** @type {EventListenerOrEventListenerObject} */
(this.__preventCloseOutsideClick),
(this.__onInsideMouseDown),
true,
);
this.invokerNode[addOrRemoveListener](
'mouseup',
/** @type {EventListenerOrEventListenerObject} */
(this.__onInsideMouseUp),
true,
);
}
document.documentElement[addOrRemoveListener](
'click',
'mouseup',
/** @type {EventListenerOrEventListenerObject} */
(this.__onCaptureHtmlClick),
(this.__onDocumentMouseUp),
true,
);
}

1
packages/overlays/test-helpers.js Normal file
View file

@ -0,0 +1 @@
export { mimicClick } from './test-helpers/mimicClick.js';

21
packages/overlays/test-helpers/mimicClick.js Normal file
View file

@ -0,0 +1,21 @@
async function sleep(t = 0) {
return new Promise(resolve => {
setTimeout(() => {
resolve(true);
}, t);
});
}
/**
* @param {HTMLElement} el
* @param {{isAsync?:boolean, releaseElement?: HTMLElement}} [config]
*/
export async function mimicClick(el, { isAsync, releaseElement } = { isAsync: false }) {
const releaseEl = releaseElement || el;
el.dispatchEvent(new MouseEvent('mousedown'));
if (isAsync) {
await sleep();
}
releaseEl.dispatchEvent(new MouseEvent('click'));
releaseEl.dispatchEvent(new MouseEvent('mouseup'));
}

96
packages/overlays/test/OverlayController.test.js
View file

@ -8,6 +8,7 @@ import { OverlayController } from '../src/OverlayController.js';
import { overlays } from '../src/overlays.js';
import { keyCodes } from '../src/utils/key-codes.js';
import { simulateTab } from '../src/utils/simulate-tab.js';
import { mimicClick } from '../test-helpers.js';
/**
* @typedef {import('../types/OverlayConfig').OverlayConfig} OverlayConfig
@ -451,8 +452,12 @@ describe('OverlayController', () => {
contentNode,
});
await ctrl.show();
mimicClick(document.body);
await aTimeout(0);
expect(ctrl.isShown).to.be.false;
document.body.click();
await ctrl.show();
await mimicClick(document.body, { isAsync: true });
await aTimeout(0);
expect(ctrl.isShown).to.be.false;
});
@ -479,6 +484,13 @@ describe('OverlayController', () => {
expect(ctrl.isShown).to.be.true;
// Don't hide on inside mousedown & outside mouseup
ctrl.contentNode.dispatchEvent(new MouseEvent('mousedown'));
await aTimeout(0);
document.body.dispatchEvent(new MouseEvent('mouseup'));
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
// Important to check if it can be still shown after, because we do some hacks inside
await ctrl.hide();
expect(ctrl.isShown).to.be.false;
@ -486,6 +498,42 @@ describe('OverlayController', () => {
expect(ctrl.isShown).to.be.true;
});
it('only hides when both mousedown and mouseup events are outside', async () => {
const contentNode = /** @type {HTMLElement} */ (await fixture('<div>Content</div>'));
const ctrl = new OverlayController({
...withGlobalTestConfig(),
hidesOnOutsideClick: true,
contentNode,
invokerNode: /** @type {HTMLElement} */ (fixtureSync(html`
<div role="button" style="width: 100px; height: 20px;">Invoker</div>
`)),
});
await ctrl.show();
mimicClick(document.body, { releaseElement: contentNode });
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
mimicClick(contentNode, { releaseElement: document.body });
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
mimicClick(document.body, {
releaseElement: /** @type {HTMLElement} */ (ctrl.invokerNode),
});
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
mimicClick(/** @type {HTMLElement} */ (ctrl.invokerNode), {
releaseElement: document.body,
});
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
mimicClick(document.body);
await aTimeout(0);
expect(ctrl.isShown).to.be.false;
});
it('doesn\'t hide on "inside sub shadow dom" click', async () => {
const invokerNode = /** @type {HTMLElement} */ (await fixture('<button>Invoker</button>'));
const contentNode = /** @type {HTMLElement} */ (await fixture('<div>Content</div>'));
@ -548,22 +596,14 @@ describe('OverlayController', () => {
contentNode,
invokerNode,
});
const stopProp = (/** @type {Event} */ e) => e.stopPropagation();
const dom = await fixture(
/**
* @param {{ stopPropagation: () => any; }} e
*/
`
<div>
<div id="popup">${invokerNode}${contentNode}</div>
<div
id="regular-sibling"
@click="${() => {
/* propagates */
}}"
></div>
<third-party-noise @click="${(/** @type {Event} */ e) => e.stopPropagation()}">
<div id="third-party-noise" @click="${stopProp}" @mousedown="${stopProp}" @mouseup="${stopProp}">
This element prevents our handlers from reaching the document click handler.
</third-party-noise>
</div>
</div>
`,
);
@ -571,8 +611,9 @@ describe('OverlayController', () => {
await ctrl.show();
expect(ctrl.isShown).to.equal(true);
/** @type {HTMLElement} */
(dom.querySelector('third-party-noise')).click();
const noiseEl = /** @type {HTMLElement} */ (dom.querySelector('#third-party-noise'));
mimicClick(noiseEl);
await aTimeout(0);
expect(ctrl.isShown).to.equal(false);
@ -592,35 +633,26 @@ describe('OverlayController', () => {
contentNode,
invokerNode,
});
const stopProp = (/** @type {Event} */ e) => e.stopPropagation();
const dom = /** @type {HTMLElement} */ (await fixture(`
<div>
<div id="popup">${invokerNode}${ctrl.content}</div>
<div
id="regular-sibling"
@click="${() => {
/* propagates */
}}"
></div>
<third-party-noise>
<div id="third-party-noise">
This element prevents our handlers from reaching the document click handler.
</third-party-noise>
</div>
</div>
`));
/** @type {HTMLElement} */
(dom.querySelector('third-party-noise')).addEventListener(
'click',
(/** @type {Event} */ event) => {
event.stopPropagation();
},
true,
);
const noiseEl = /** @type {HTMLElement} */ (dom.querySelector('#third-party-noise'));
noiseEl.addEventListener('click', stopProp, true);
noiseEl.addEventListener('mousedown', stopProp, true);
noiseEl.addEventListener('mouseup', stopProp, true);
await ctrl.show();
expect(ctrl.isShown).to.equal(true);
/** @type {HTMLElement} */
(dom.querySelector('third-party-noise')).click();
mimicClick(noiseEl);
await aTimeout(0);
expect(ctrl.isShown).to.equal(false);

6
packages/select-rich/test/lion-select-rich.test.js
View file

@ -2,6 +2,7 @@ import { LitElement } from '@lion/core';
import { renderLitAsNode } from '@lion/helpers';
import { OverlayController } from '@lion/overlays';
import { LionOption } from '@lion/listbox';
import { mimicClick } from '@lion/overlays/test-helpers';
import {
aTimeout,
defineCE,
@ -253,9 +254,8 @@ describe('lion-select-rich', () => {
));
expect(el.opened).to.be.true;
// a click on the button will trigger hide on outside click
// which we then need to sync back to "opened"
outerEl.click();
mimicClick(outerEl);
await aTimeout(0);
expect(el.opened).to.be.false;
});