Error with event bubbling #15

silvenon · 2015-07-26T17:16:40Z

There's an error happening when a parent of the child which is closing the portal has an onClick handler:

import jsdom from 'jsdom';
import sinon from 'sinon';
import assert from 'assert';

describe('Portal', () => {
  let React, utils, Portal;

  before(() => {
    global.document = jsdom.jsdom('<!DOCTYPE html><html><head></head><body></body></html>');
    global.window = global.document.parentWindow;
    global.navigator = window.navigator;

    React = require('react/addons');
    utils = React.addons.TestUtils;
    Portal = require('react-portal');
  });

  it('handles event bubbling after close', () => {
    const cb = sinon.spy();

    const Component = React.createClass({
      handleClose() {
        this.refs.portal.closePortal();
        console.log('close');
      },

      render() {
        return (
          <Portal ref="portal" openByClickOn={<span ref="toggle" />} onClose={cb}>
            <div onClick={function () { }}> // it works without the handler
              <button ref="close" type="button" onClick={this.handleClose}>Close</button>
            </div>
          </Portal>
        );
      }
    });

    const el = utils.renderIntoDocument(<Component />);

    utils.Simulate.click(React.findDOMNode(el.refs.toggle));
    utils.Simulate.click(React.findDOMNode(el.refs.close));

    assert(cb.called);
  });
});

TypeError: Cannot read property 'firstChild' of undefined

I created a demo where you can run this test.

The text was updated successfully, but these errors were encountered:

tajo · 2015-07-28T12:47:20Z

this.refs.portal.closePortal()

This is bad design and it will break with React 0.14 since refs will return underlying DOM nodes and not React components anymore.

There are two other and better ways:

Put

<div onClick={function () { }}> // it works without the handler
  <button ref="close" type="button" onClick={this.handleClose}>Close</button>
</div>

into a new component and then you can use

<button ref="close" type="button" onClick={this.props.closePortal}>Close</button>

or if you don't want to create a new component for some reason, you can use the prop isOpened:

<span onClick={(function(e) {this.setState({isOpened: true})}).bind(this)}>
<Portal isOpened={this.state.isOpened}>
  <div onClick={function () { }}> // should work now
    <button ref="close" type="button" onClick={(function(e) {this.setState({isOpened: false})}).bind(this)}>Close</button>
  </div>
</Portal>

I didn't test the code above, so I'm sorry for possible typos.

silvenon · 2015-07-28T14:42:48Z

Works perfectly, thanks!

This is bad design and it will break with React 0.14 since refs will return underlying DOM nodes and not React components anymore.

This only applies to native DOM components, not custom components:

As with refs, this change does not affect custom components (eg. <MyFancyMenu> or <MyContextProvider>), which remain unaffected by this change.

silvenon closed this as completed Jul 28, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Error with event bubbling #15

Error with event bubbling #15

silvenon commented Jul 26, 2015

tajo commented Jul 28, 2015

silvenon commented Jul 28, 2015

Error with event bubbling #15

Error with event bubbling #15

Comments

silvenon commented Jul 26, 2015

tajo commented Jul 28, 2015

silvenon commented Jul 28, 2015