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

fix: throw when passing unconnected contentNode

Browse source 
This was already not working and would give you problems, now we explicitly mention why it fails by throwing an error
This commit is contained in:
Joren Broekema 2020-05-28 14:46:21 +02:00 committed by Thomas Allmer
parent 5f55c1d3c5
commit 78dbfbed49
4 changed files with 50 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
packages/overlays/docs/20-index.md
View file

@ -59,9 +59,11 @@ and has the following public functions:
All overlays contain an invokerNode and a contentNode
- **contentNode**, the toggleable content of the overlay
- **contentNode**, the toggleable content of the overlay.
- **invokerNode**, the element toggles the visibility of the content. For local overlays, this is the relative element the content is positioned to
> Make sure you pass a DOM-connected contentNode, an offline rendered (e.g. with just `document.createElement` or `renderLitAsNode`) will not work, because then we cannot determine the renderTarget to render the content to.
For DOM position, local refers to overlays where the content is positioned next to the invokers they are related to, DOM-wise.
Global refers to overlays where the content is positioned in a global root node at the bottom of `<body>`.

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

@ -135,6 +135,11 @@ export class OverlayController {
this._contentId = `overlay-content--${Math.random().toString(36).substr(2, 10)}`;
if (this._defaultConfig.contentNode) {
if (!this._defaultConfig.contentNode.isConnected) {
throw new Error(
'[OverlayController] Could not find a render target, since the provided contentNode is not connected to the DOM. Make sure that it is connected, e.g. by doing "document.body.appendChild(contentNode)", before passing it on.',
);
}
this.__isContentNodeProjected = Boolean(this._defaultConfig.contentNode.assignedSlot);
}
this.updateConfig(config);

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

@ -1,20 +1,20 @@
/* eslint-disable no-new */
import '@lion/core/test-helpers/keyboardEventShimIE.js';
import {
expect,
html,
fixture,
aTimeout,
defineCE,
unsafeStatic,
expect,
fixture,
html,
nextFrame,
unsafeStatic,
} from '@open-wc/testing';
import { fixtureSync } from '@open-wc/testing-helpers';
import '@lion/core/test-helpers/keyboardEventShimIE.js';
import sinon from 'sinon';
import { keyCodes } from '../src/utils/key-codes.js';
import { simulateTab } from '../src/utils/simulate-tab.js';
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';
const withGlobalTestConfig = () => ({
placementMode: 'global',
@ -153,6 +153,30 @@ describe('OverlayController', () => {
});
expect(ctrl._renderTarget).to.equal(parentNode);
});
it('throws when passing a content node that was created "offline"', async () => {
const contentNode = document.createElement('div');
const createOverlayController = () => {
new OverlayController({
...withLocalTestConfig(),
contentNode,
});
};
expect(createOverlayController).to.throw(
'[OverlayController] Could not find a render target, since the provided contentNode is not connected to the DOM. Make sure that it is connected, e.g. by doing "document.body.appendChild(contentNode)", before passing it on.',
);
});
it('succeeds when passing a content node that was created "online"', async () => {
const contentNode = document.createElement('div');
document.body.appendChild(contentNode);
const overlay = new OverlayController({
...withLocalTestConfig(),
contentNode,
});
expect(overlay.contentNode.isConnected).to.be.true;
expect(overlay._renderTarget).to.not.be.undefined;
});
});
});
@ -200,6 +224,9 @@ describe('OverlayController', () => {
contentNode.slot = 'contentNode';
shadowHost.appendChild(contentNode);
// Ensure the contentNode is connected to DOM
document.body.appendChild(shadowHost);
const ctrl = new OverlayController({
...withLocalTestConfig(),
contentNode,
@ -1210,9 +1237,12 @@ describe('OverlayController', () => {
describe('Exception handling', () => {
it('throws if no .placementMode gets passed on', async () => {
const contentNode = document.createElement('div');
// Ensure the contentNode is connected to DOM
document.body.appendChild(contentNode);
expect(() => {
new OverlayController({
contentNode: {},
contentNode,
});
}).to.throw('You need to provide a .placementMode ("global"|"local")');
});
@ -1246,6 +1276,9 @@ describe('OverlayController', () => {
contentNode.slot = 'contentNode';
shadowHost.appendChild(contentNode);
// Ensure the contentNode is connected to DOM
document.body.appendChild(shadowHost);
expect(() => {
new OverlayController({
...withLocalTestConfig(),

5
packages/overlays/test/OverlaysManager.test.js
View file

@ -6,16 +6,13 @@ describe('OverlaysManager', () => {
let defaultOptions;
let mngr;
before(async () => {
beforeEach(async () => {
const contentNode = await fixture(html`<p>my content</p>`);
defaultOptions = {
placementMode: 'global',
contentNode,
};
});
beforeEach(() => {
mngr = new OverlaysManager();
});