Passing an element vs. a react class to an optionally displayed modal

919 Views Asked by At

I have a table cell that the user can click on which will launch a react-bootstrap modal that displays additional details. The modal displays a component that has its own state and may trigger an action to pull data from the back-end if the store doesn't have the data it needs.

Currently I'm passing the component as a react element to react-bootstrap's OverlayMixin to show the details in the modal but I'm wondering if instead I should be passing a react class and rendering it with React.createElement.

Current code:

var MyCell = React.creatClass({
    _renderDetails: function () { return (<Details id={this.props.id}/>); 

    render: function() {
        return (
            <td>
                <MyModal modal={this._renderDetails()}>
                    {this.props.value}
                </MyModal>
            </td>
        );
     }
});

var MyModal = React.createClass({
    props: { content: React.PropTypes.element.isRequired }
    mixins: [OverlayMixin],

    // Called by the OverlayMixin when this component is mounted or updated.
    renderOverlay: function() {
        if (!this.state.isModalOpen) { return (<span/>); }
        return (
            <Modal className='information-modal' onRequestHide={this.handleToggle}>
                <div className='modal-body'>
                    {this.props.modal}
                </div>
            </Modal>
        );
    }
});

The reason I ask this question is because I was looking at Griddle's documentation they appear to be passing a react class instead.

var LinkComponent = React.createClass({
    render: function() { return <a href ... </a> } 
});
var columnMeta = [ { "customComponent": LinkComponent };

And then rendering it using React.CreateElement. Source Code

var colData = (<meta.customComponent data= ... />;
returnValue = <td>{colData}</td>);

// Turns into
var colData = React.createElement(meta.customComponent, {data: ...
returnValue = React.createElement("td" ... colData));
2

There are 2 best solutions below

0
On BEST ANSWER

Since Griddle uses the customComponent property to render each item in a column, it must be a ReactComponent class, otherwise it would render the exact same component for each row. Said another way, customComponent represents a template to create new ReactElement from. (I personally would prefer to be able to pass a function that receives the appropriate properties and returns an ReactElement.)

In your case, you only need to specify a single component, so passing a ReactElement makes sense. It's also more powerful from an end-user perspective, because you can create a component that is wired up to its parent. This is also a very common pattern when using this.props.children.

0
On

First off, I'd be hesitant to pass JSX code around like this. React.js gives you the tools for meta-programming -- injecting event handlers and markup into child components is very easy to do, and can often seem like the simplest way to do things. That way lies ruin, IMO. Dynamically constructed components like this are always tough to debug, since its hard to trace where anything is coming from. props and state are better suited for passing simpler data structures around, offloading the actual rendering logic to interested components.

A fair criticism of this point might be, "this is how libraries like MaterialUI and ReactBootstrap do it! They inject callbacks and markup everywhere." Well, they are building abstract interfaces and a UI for developers. We have the luxury of building concrete interfaces that can trade abstraction for clarity and readability. We don't have to abstract everything all the time.

That said, to your specific problem: I like the idea of passing a component over a chunk of JSX code, since the former gives the child component control over how to render it. MyModal should be in charge of rendering its children, not asking its parent (through props) to do the work instead. This has the added benefit of allowing MyModal to change Details after a state change -- right now, its simply receiving static markup to render.