From ecdb177af2bbbd8119c24f9db7e59ce508bf8368 Mon Sep 17 00:00:00 2001 From: Robert-Frampton Date: Thu, 4 Jan 2018 08:09:39 -0800 Subject: [PATCH 1/3] Adds portalElement config property to Component. Closes #330 --- packages/metal-component/src/Component.js | 40 +++++++++++++++++++ .../src/IncrementalDomRenderer.js | 4 +- .../src/cleanup/unused.js | 4 +- .../src/render/render.js | 5 ++- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/metal-component/src/Component.js b/packages/metal-component/src/Component.js index 80cc808d..d72688fb 100644 --- a/packages/metal-component/src/Component.js +++ b/packages/metal-component/src/Component.js @@ -132,6 +132,13 @@ class Component extends EventEmitter { */ this.initialConfig_ = config || {}; + /** + * Indicates whether the component should be rendered as a Portal, outside + * of the parent component. + * @type {string|Element|boolean} + */ + this.portalElement = null; + /** * Whether the element was rendered. * @type {boolean} @@ -154,6 +161,8 @@ class Component extends EventEmitter { this.setUpDataManager_(); this.setUpSyncUpdates_(); + this.setUpPortal_(this.initialConfig_.portalElement); + this.on('stateWillChange', this.handleStateWillChange_); this.on('stateChanged', this.handleComponentStateChanged_); this.on('eventsChanged', this.onEventsChanged_); @@ -658,6 +667,37 @@ class Component extends EventEmitter { ); } + /** + * Overwrites element property if portalElement is passed. Creates + * a nested placeholder so that portalElement is not removed from the + * DOM when component first renders. When portalElement is equal to true, + * component is appeneded to the body. + * + * @param {string|Element|boolean} portalElement + */ + setUpPortal_(portalElement) { + if ( + !isElement(portalElement) && + !isString(portalElement) && + !isBoolean(portalElement) + ) { + return; + } else if (isBoolean(portalElement) && portalElement) { + portalElement = 'body'; + } + + portalElement = toElement(portalElement); + + if (portalElement) { + const placeholder = document.createElement('div'); + + portalElement.appendChild(placeholder); + + this.element = placeholder; + this.portalElement = portalElement; + } + } + /** * Sets up the component's renderer. * @protected diff --git a/packages/metal-incremental-dom/src/IncrementalDomRenderer.js b/packages/metal-incremental-dom/src/IncrementalDomRenderer.js index 8c64eaa0..2f60fba1 100644 --- a/packages/metal-incremental-dom/src/IncrementalDomRenderer.js +++ b/packages/metal-incremental-dom/src/IncrementalDomRenderer.js @@ -35,7 +35,9 @@ class IncrementalDomRenderer extends ComponentRenderer.constructor { for (let i = 0; i < data.childComponents.length; i++) { const child = data.childComponents[i]; if (!child.isDisposed()) { - child.element = null; + if (!child.portalElement) { + child.element = null; + } child.dispose(); } } diff --git a/packages/metal-incremental-dom/src/cleanup/unused.js b/packages/metal-incremental-dom/src/cleanup/unused.js index 6489a6d3..70590c66 100644 --- a/packages/metal-incremental-dom/src/cleanup/unused.js +++ b/packages/metal-incremental-dom/src/cleanup/unused.js @@ -20,7 +20,9 @@ export function disposeUnused() { if (!comp.isDisposed() && !getData(comp).parent) { // Don't let disposing cause the element to be removed, since it may // be currently being reused by another component. - comp.element = null; + if (!comp.portalElement) { + comp.element = null; + } comp.dispose(); } } diff --git a/packages/metal-incremental-dom/src/render/render.js b/packages/metal-incremental-dom/src/render/render.js index 7850128f..a290907b 100644 --- a/packages/metal-incremental-dom/src/render/render.js +++ b/packages/metal-incremental-dom/src/render/render.js @@ -537,7 +537,10 @@ function renderSubComponent_(tagOrCtor, config, owner) { config.key = parentData.config.key; } - comp.getRenderer().renderInsidePatch(comp); + if (!comp.portalElement) { + comp.getRenderer().renderInsidePatch(comp); + } + if (!comp.wasRendered) { comp.renderComponent(); } From 219f42961b8c46ba4dee7eeb0d9bd10fb48739d6 Mon Sep 17 00:00:00 2001 From: Robert-Frampton Date: Thu, 4 Jan 2018 14:58:13 -0800 Subject: [PATCH 2/3] Add test cases for portalElement --- packages/metal-component/test/Component.js | 54 +++++ .../test/IncrementalDomRenderer.js | 213 ++++++++++++++++++ 2 files changed, 267 insertions(+) diff --git a/packages/metal-component/test/Component.js b/packages/metal-component/test/Component.js index 7e751b2c..2f21c319 100644 --- a/packages/metal-component/test/Component.js +++ b/packages/metal-component/test/Component.js @@ -1152,6 +1152,60 @@ describe('Component', function() { sinon.assert.calledWith(hookStub, comp); }); + it('should render component to portalElement', function() { + const portalElement = document.createElement('span'); + + comp = Component.render(Component, { + portalElement, + }); + + assert.strictEqual(comp.element.parentNode, portalElement); + assert.strictEqual(comp.portalElement, portalElement); + }); + + it('should set portalElement from selector', function() { + const portalElement = document.createElement('div'); + portalElement.setAttribute('id', 'foo'); + + document.body.appendChild(portalElement); + + comp = Component.render(Component, { + portalElement: '#foo', + }); + + assert.strictEqual(comp.element.parentNode, portalElement); + }); + + it('should set portalElement to body when set to true', function() { + const parentElement = document.createElement('span'); + + document.body.appendChild(parentElement); + + comp = Component.render( + Component, + { + portalElement: true, + }, + parentElement + ); + + assert.strictEqual(comp.element.parentNode, document.body); + }); + + it('should detach component from DOM when portalElement is passed', function() { + const portalElement = document.createElement('span'); + + comp = Component.render(Component, { + portalElement, + }); + + assert.ok(comp.inDocument); + + comp.dispose(); + + assert.ok(!comp.inDocument); + }); + function createCustomComponentClass(rendererContentOrFn) { class CustomComponent extends Component {} CustomComponent.RENDERER = createCustomRenderer(rendererContentOrFn); diff --git a/packages/metal-incremental-dom/test/IncrementalDomRenderer.js b/packages/metal-incremental-dom/test/IncrementalDomRenderer.js index 5687f07d..9857053a 100644 --- a/packages/metal-incremental-dom/test/IncrementalDomRenderer.js +++ b/packages/metal-incremental-dom/test/IncrementalDomRenderer.js @@ -3331,4 +3331,217 @@ describe('IncrementalDomRenderer', function() { const config = IncrementalDomRenderer.getConfig(component); assert.strictEqual(getData(component).config, config); }); + + describe('Portals', function() { + beforeEach(function() { + document.body.innerHTML = ''; + }); + + it('should render sub components to defined portalElement', function() { + const portalElement = createPortalElement(); + + class TestChildComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Child'); + IncDom.elementClose('div'); + } + } + TestChildComponent.RENDERER = IncrementalDomRenderer; + + class TestComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Parent'); + IncDom.elementOpen( + TestChildComponent, + null, + null, + 'ref', + 'child', + 'portalElement', + portalElement + ); + IncDom.elementClose(TestChildComponent); + IncDom.elementClose('div'); + } + } + TestComponent.RENDERER = IncrementalDomRenderer; + + component = new TestComponent(); + + assert.strictEqual( + document.body.innerHTML, + '
Child
Parent
' + ); + }); + + it('should update sub components that have a defined portalElement', function( + done + ) { + const portalElement = createPortalElement(); + + class TestChildComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Child: ' + this.foo); + IncDom.elementClose('div'); + } + } + TestChildComponent.RENDERER = IncrementalDomRenderer; + TestChildComponent.STATE = { + foo: {}, + }; + + class TestComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Parent: ' + this.foo); + IncDom.elementOpen( + TestChildComponent, + null, + null, + 'ref', + 'child', + 'foo', + this.foo, + 'portalElement', + portalElement + ); + IncDom.elementClose(TestChildComponent); + IncDom.elementClose('div'); + } + } + TestComponent.RENDERER = IncrementalDomRenderer; + TestComponent.STATE = { + foo: { + value: 'bar', + }, + }; + + component = new TestComponent(); + + assert.strictEqual( + document.body.innerHTML, + '
Child: bar
Parent: bar
' + ); + + component.foo = 'baz'; + component.refs.child.once('stateSynced', function() { + assert.strictEqual( + document.body.innerHTML, + '
Child: baz
Parent: baz
' + ); + done(); + }); + }); + + it('should dispose sub components with a portalElement when parent is disposed', function() { + const portalElement = createPortalElement(); + + class TestChildComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Child'); + IncDom.elementClose('div'); + } + } + TestChildComponent.RENDERER = IncrementalDomRenderer; + + class TestComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Parent'); + IncDom.elementOpen( + TestChildComponent, + null, + null, + 'ref', + 'child', + 'portalElement', + portalElement + ); + IncDom.elementClose(TestChildComponent); + IncDom.elementClose('div'); + } + } + TestComponent.RENDERER = IncrementalDomRenderer; + + component = new TestComponent(); + + assert.strictEqual( + document.body.innerHTML, + '
Child
Parent
' + ); + + component.dispose(); + + assert.strictEqual(document.body.innerHTML, '
'); + }); + + it('should dispose sub components with portalElement when removed by parent component', function( + done + ) { + const portalElement = createPortalElement(); + + class TestChildComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Child'); + IncDom.elementClose('div'); + } + } + TestChildComponent.RENDERER = IncrementalDomRenderer; + + class TestComponent extends Component { + render() { + IncDom.elementOpen('div'); + IncDom.text('Parent'); + if (!this.remove) { + IncDom.elementVoid( + TestChildComponent, + null, + null, + 'ref', + 'child', + 'portalElement', + portalElement + ); + } + IncDom.elementClose('div'); + } + } + TestComponent.RENDERER = IncrementalDomRenderer; + TestComponent.STATE = { + remove: { + value: false, + }, + }; + + component = new TestComponent(); + + assert.strictEqual( + document.body.innerHTML, + '
Child
Parent
' + ); + + component.remove = true; + + component.refs.child.once('disposed', function() { + assert.strictEqual( + document.body.innerHTML, + '
Parent
' + ); + + done(); + }); + }); + }); }); + +function createPortalElement() { + const portalElement = document.createElement('div'); + portalElement.setAttribute('id', 'host'); + document.body.appendChild(portalElement); + return portalElement; +} From 6bd1caa1223959daf29109520810346b7dd3fe08 Mon Sep 17 00:00:00 2001 From: Robert-Frampton Date: Thu, 4 Jan 2018 16:25:02 -0800 Subject: [PATCH 3/3] Skip portal rendering during SSR --- packages/metal-component/src/Component.js | 5 +++++ .../metal-incremental-dom/src/render/render.js | 5 +++++ .../metal-isomorphic/test/fixtures/Portal.js | 16 ++++++++++++++++ .../metal-isomorphic/test/fixtures/Portal.soy | 10 ++++++++++ .../test/fixtures/PortalParent.js | 16 ++++++++++++++++ .../test/fixtures/PortalParent.soy | 15 +++++++++++++++ packages/metal-isomorphic/test/isomorphic.js | 13 ++++++++++++- 7 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 packages/metal-isomorphic/test/fixtures/Portal.js create mode 100644 packages/metal-isomorphic/test/fixtures/Portal.soy create mode 100644 packages/metal-isomorphic/test/fixtures/PortalParent.js create mode 100644 packages/metal-isomorphic/test/fixtures/PortalParent.soy diff --git a/packages/metal-component/src/Component.js b/packages/metal-component/src/Component.js index d72688fb..10a5a0ba 100644 --- a/packages/metal-component/src/Component.js +++ b/packages/metal-component/src/Component.js @@ -686,6 +686,11 @@ class Component extends EventEmitter { portalElement = 'body'; } + if (isServerSide()) { + this.portalElement = true; + return; + } + portalElement = toElement(portalElement); if (portalElement) { diff --git a/packages/metal-incremental-dom/src/render/render.js b/packages/metal-incremental-dom/src/render/render.js index a290907b..49dee270 100644 --- a/packages/metal-incremental-dom/src/render/render.js +++ b/packages/metal-incremental-dom/src/render/render.js @@ -17,6 +17,7 @@ import { isDef, isDefAndNotNull, isFunction, + isServerSide, isString, object, } from 'metal'; @@ -537,6 +538,10 @@ function renderSubComponent_(tagOrCtor, config, owner) { config.key = parentData.config.key; } + if (comp.portalElement && isServerSide()) { + return comp; + } + if (!comp.portalElement) { comp.getRenderer().renderInsidePatch(comp); } diff --git a/packages/metal-isomorphic/test/fixtures/Portal.js b/packages/metal-isomorphic/test/fixtures/Portal.js new file mode 100644 index 00000000..80d45217 --- /dev/null +++ b/packages/metal-isomorphic/test/fixtures/Portal.js @@ -0,0 +1,16 @@ +import Component from 'metal-component'; +import Soy from 'metal-soy'; +import './ChildComponent.soy.js'; + +import templates from './Portal.soy.js'; + +class Portal extends Component { +} + +Portal.STATE = { + message: {} +}; + +Soy.register(Portal, templates); + +export default Portal; diff --git a/packages/metal-isomorphic/test/fixtures/Portal.soy b/packages/metal-isomorphic/test/fixtures/Portal.soy new file mode 100644 index 00000000..736ba38a --- /dev/null +++ b/packages/metal-isomorphic/test/fixtures/Portal.soy @@ -0,0 +1,10 @@ +{namespace Portal} + +/** + * @param? message + */ +{template .render} +
+ Portal: {$message} +
+{/template} diff --git a/packages/metal-isomorphic/test/fixtures/PortalParent.js b/packages/metal-isomorphic/test/fixtures/PortalParent.js new file mode 100644 index 00000000..12c15879 --- /dev/null +++ b/packages/metal-isomorphic/test/fixtures/PortalParent.js @@ -0,0 +1,16 @@ +import Component from 'metal-component'; +import Soy from 'metal-soy'; +import './Portal.soy.js'; + +import templates from './PortalParent.soy.js'; + +class PortalParent extends Component { +} + +PortalParent.STATE = { + message: {} +}; + +Soy.register(PortalParent, templates); + +export default PortalParent; diff --git a/packages/metal-isomorphic/test/fixtures/PortalParent.soy b/packages/metal-isomorphic/test/fixtures/PortalParent.soy new file mode 100644 index 00000000..9732d7f8 --- /dev/null +++ b/packages/metal-isomorphic/test/fixtures/PortalParent.soy @@ -0,0 +1,15 @@ +{namespace PortalParent} + +/** + * @param? message + */ +{template .render} +
+ Parent: {$message} + + {call Portal.render} + {param message: $message /} + {param portalElement: true /} + {/call} +
+{/template} diff --git a/packages/metal-isomorphic/test/isomorphic.js b/packages/metal-isomorphic/test/isomorphic.js index 919e6dbe..ef08e625 100644 --- a/packages/metal-isomorphic/test/isomorphic.js +++ b/packages/metal-isomorphic/test/isomorphic.js @@ -2,8 +2,9 @@ import Component from 'metal-component'; import MyComponent from './fixtures/MyComponent'; import MyJSXComponent from './fixtures/MyJSXComponent'; import ParentComponent from './fixtures/ParentComponent'; -import {assert} from 'chai'; +import PortalParent from './fixtures/PortalParent'; import jsdomGlobal from 'jsdom-global'; +import {assert} from 'chai'; describe('Isomorphic Rendering', () => { it('should render soy component to string', () => { @@ -36,6 +37,16 @@ describe('Isomorphic Rendering', () => { assert.equal(htmlString, '
Child: Hello, World!
'); }); + it('should skip subcomponents that have portalElement defined', () => { + assert.ok(!global.document); + + const htmlString = Component.renderToString(PortalParent, { + message: 'Hello, World!' + }); + + assert.equal(htmlString, '
Parent: Hello, World!
'); + }); + describe('JSDom', () => { let cleanup;