Skip to content

Commit

Permalink
Merge pull request #222 from react-bootstrap/overlayInstance
Browse files Browse the repository at this point in the history 
Drop busted getOverlayDOMNode
  • Loading branch information
@taion
taion committed Oct 24, 2017
1 parent 29d54c7 commit fe1ee5e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 75 deletions.
24 changes: 5 additions & 19 deletions src/LegacyPortal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import ownerDocument from './utils/ownerDocument';
* The children of `<Portal/>` component will be appended to the `container` specified.
*/
class Portal extends React.Component {

static displayName = 'Portal';

static propTypes = {
Expand Down Expand Up @@ -58,15 +57,15 @@ class Portal extends React.Component {
this._portalContainerNode = getContainer(this.props.container, ownerDocument(this).body);
this._portalContainerNode.appendChild(this._overlayTarget);
}
}
};

_unmountOverlayTarget = () => {
if (this._overlayTarget) {
this._portalContainerNode.removeChild(this._overlayTarget);
this._overlayTarget = null;
}
this._portalContainerNode = null;
}
};

_renderOverlay = () => {
let overlay = !this.props.children
Expand All @@ -91,35 +90,22 @@ class Portal extends React.Component {
this._unrenderOverlay();
this._unmountOverlayTarget();
}
}
};

_unrenderOverlay = () => {
if (this._overlayTarget) {
ReactDOM.unmountComponentAtNode(this._overlayTarget);
this._overlayInstance = null;
}
}
};

render() {
return null;
}

getMountNode = () => {
return this._overlayTarget;
}

getOverlayDOMNode = () => {
if (!this._isMounted) {
throw new Error('getOverlayDOMNode(): A component must be mounted to have a DOM node.');
}

if (this._overlayInstance) {
return ReactDOM.findDOMNode(this._overlayInstance);
}

return null;
}

};
}

export default Portal;
25 changes: 6 additions & 19 deletions src/Portal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import LegacyPortal from './LegacyPortal'
* The children of `<Portal/>` component will be appended to the `container` specified.
*/
class Portal extends React.Component {

static displayName = 'Portal';

static propTypes = {
Expand All @@ -31,8 +30,7 @@ class Portal extends React.Component {
};

componentDidMount() {
this._isMounted = true;
this.setContainer()
this.setContainer();
this.forceUpdate(this.props.onRendered)
}

Expand All @@ -43,13 +41,14 @@ class Portal extends React.Component {
}

componentWillUnmount() {
this._isMounted = false;
this._portalContainerNode = null;
}

setContainer = (props = this.props) => {
this._portalContainerNode = getContainer(props.container, ownerDocument(this).body);
}
this._portalContainerNode = getContainer(
props.container, ownerDocument(this).body,
);
};

render() {
return this.props.children && this._portalContainerNode ?
Expand All @@ -59,19 +58,7 @@ class Portal extends React.Component {

getMountNode = () => {
return this._portalContainerNode;
}

getOverlayDOMNode = () => {
if (!this._isMounted) {
throw new Error('getOverlayDOMNode(): A component must be mounted to have a DOM node.');
}

if (this._overlayInstance) {
return ReactDOM.findDOMNode(this._overlayInstance);
}

return null;
}
};
}

export default ReactDOM.createPortal ? Portal : LegacyPortal;
24 changes: 10 additions & 14 deletions test/LegacyPortalSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import ReactTestUtils from 'react-dom/test-utils';
import Portal from '../src/LegacyPortal';

describe('LegacyPortal', () => {
let instance;

class Overlay extends React.Component {
render() {
return (
Expand All @@ -20,16 +18,12 @@ describe('LegacyPortal', () => {
</div>
);
}

getOverlayDOMNode = () => {
return this.portal.getOverlayDOMNode();
}
}

it('should render overlay into container (DOMNode)', () => {
let container = document.createElement('div');
const container = document.createElement('div');

instance = ReactTestUtils.renderIntoDocument(
ReactTestUtils.renderIntoDocument(
<Overlay container={container} overlay={<div id="test1" />} />
);

Expand All @@ -43,11 +37,13 @@ describe('LegacyPortal', () => {
}
}

instance = ReactTestUtils.renderIntoDocument(
const instance = ReactTestUtils.renderIntoDocument(
<Container />
);

assert.equal(ReactDOM.findDOMNode(instance).querySelectorAll('#test1').length, 1);
expect(
ReactDOM.findDOMNode(instance).querySelectorAll('#test1')
).to.have.lengthOf(1)
});

it('should not render a null overlay', () => {
Expand All @@ -63,11 +59,11 @@ describe('LegacyPortal', () => {
}
}

instance = ReactTestUtils.renderIntoDocument(
const instance = ReactTestUtils.renderIntoDocument(
<Container />
);

assert.equal(instance.overlay.getOverlayDOMNode(), null);
expect(ReactDOM.findDOMNode(instance).childNodes).to.be.empty;
});


Expand All @@ -90,7 +86,7 @@ describe('LegacyPortal', () => {
}
}

let overlayInstance = ReactTestUtils.renderIntoDocument(
const overlayInstance = ReactTestUtils.renderIntoDocument(
<ContainerTest overlay={<div id="test1" />} />
);

Expand Down Expand Up @@ -128,7 +124,7 @@ describe('LegacyPortal', () => {
}
}

instance = ReactTestUtils.renderIntoDocument(
const instance = ReactTestUtils.renderIntoDocument(
<Parent />
);

Expand Down
41 changes: 18 additions & 23 deletions test/PortalSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import ReactTestUtils from 'react-dom/test-utils';

import Portal from '../src/Portal';

describe('Portal', function () {
let instance;

describe('Portal', () => {
class Overlay extends React.Component {
render() {
return (
Expand All @@ -20,37 +18,35 @@ describe('Portal', function () {
</div>
);
}

getOverlayDOMNode = () => {
return this.portal.getOverlayDOMNode();
}
}

it('Should render overlay into container (DOMNode)', function() {
let container = document.createElement('div');
it('should render overlay into container (DOMNode)', () => {
const container = document.createElement('div');

instance = ReactTestUtils.renderIntoDocument(
ReactTestUtils.renderIntoDocument(
<Overlay container={container} overlay={<div id="test1" />} />
);

assert.equal(container.querySelectorAll('#test1').length, 1);
});

it('Should render overlay into container (ReactComponent)', function() {
it('should render overlay into container (ReactComponent)', () => {
class Container extends React.Component {
render() {
return <Overlay container={this} overlay={<div id="test1" />} />;
}
}

instance = ReactTestUtils.renderIntoDocument(
const instance = ReactTestUtils.renderIntoDocument(
<Container />
);

assert.equal(ReactDOM.findDOMNode(instance).querySelectorAll('#test1').length, 1);
expect(
ReactDOM.findDOMNode(instance).querySelectorAll('#test1')
).to.have.lengthOf(1)
});

it('Should not render a null overlay', function() {
it('should not fail to render a null overlay', () => {
class Container extends React.Component {
render() {
return (
Expand All @@ -63,15 +59,14 @@ describe('Portal', function () {
}
}

instance = ReactTestUtils.renderIntoDocument(
const instance = ReactTestUtils.renderIntoDocument(
<Container />
);

assert.equal(instance.overlay.getOverlayDOMNode(), null);
expect(ReactDOM.findDOMNode(instance).childNodes).to.be.empty;
});


it('Should change container on prop change', function() {
it('should change container on prop change', () => {
class ContainerTest extends React.Component {
state = {};
render() {
Expand All @@ -90,20 +85,20 @@ describe('Portal', function () {
}
}

let overlayInstance = ReactTestUtils.renderIntoDocument(
<ContainerTest overlay={<div id="test1" />} />
const overlayInstance = ReactTestUtils.renderIntoDocument(
<ContainerTest overlay={<div id="test1" />} />,
);

assert.equal(overlayInstance.portal._portalContainerNode.nodeName, 'BODY');
overlayInstance.setState({container: overlayInstance.container})
overlayInstance.setState({container: overlayInstance.container});
assert.equal(overlayInstance.portal._portalContainerNode.nodeName, 'DIV');

ReactDOM.unmountComponentAtNode(
ReactDOM.findDOMNode(overlayInstance).parentNode,
);
});

it('Should unmount when parent unmounts', function() {
it('should unmount when parent unmounts', () => {
class Parent extends React.Component {
state = {show: true};
render() {
Expand All @@ -128,7 +123,7 @@ describe('Portal', function () {
}
}

instance = ReactTestUtils.renderIntoDocument(
const instance = ReactTestUtils.renderIntoDocument(
<Parent />
);

Expand Down

0 comments on commit fe1ee5e

Please sign in to comment.