-
Notifications
You must be signed in to change notification settings - Fork 222
Allow specifying table rows in custom components #196
base: master
Are you sure you want to change the base?
Conversation
Further commentary: Because we're doing a render ourselves of the sub-component as a means of parsing out its child components, that sub-component's |
This is boss. Just gave a it a quick look. I'll give it a shot in one of my projects and let you know. Also you don't need to keep the original implementation spike ;) |
4abe39c
to
1eb6eb9
Compare
for posterity, and as a note to myself, the commit that you commented on and then I force-pushed away: 4abe39c |
@growlsworth I think 1f02dbd should allow passing through child context, do you mind taking a look? |
In the parent, which holds the |
I'm looking at your context test. Thanks for adding that. And that looks good. I must be doing something weird. I'll check back again. |
Ok. I got the test example to pass in my machine. FYI, the |
Ok I got it to fail. The issue is in the child's child's context. :( Drop this bad boy in <!DOCTYPE html>
<html>
<head>
<title>Reactable Context Text</title>
<script type="text/javascript" src="../node_modules/react/dist/react.js"></script>
<script type="text/javascript" src="../build/reactable.js"></script>
<script type="text/javascript" src="../node_modules/grunt-babel/node_modules/babel-core/browser.js"></script>
</head>
<body>
<div id="wrapper"></div>
<script type="text/babel">
let SubItem = React.createClass({
displayName: 'SubItem',
contextTypes: { test: React.PropTypes.string },
render: function(){
let text;
if (this.context)
text = this.context.test || 'no context.test';
else
text = 'no context';
return (
<div>
{text}
</div>
);
}
});
let RowComponent = React.createClass({
displayName: 'CustomComponent',
contextTypes: { test: React.PropTypes.string },
childContextTypes: { test: React.PropTypes.string },
getChildContext: function() {
return { test: 'sub-test' };
},
render: function(){
return (
<Reactable.Tr>
<Reactable.Td column="Name">{this.props.name || ''}</Reactable.Td>
<Reactable.Td column="Test">{this.context.test || ''}</Reactable.Td>
<Reactable.Td column="SubItem"><SubItem /></Reactable.Td>
</Reactable.Tr>
);
}
});
let TableComponent = React.createClass({
displayName: 'TableComponent',
childContextTypes: { test: React.PropTypes.string },
getChildContext: function() {
return { test: 'foobar' };
},
render: function() {
return (
<Reactable.Table className="table" id="table">
<RowComponent name='Griffin Smith' />
<RowComponent name='Lee Salminen' />
</Reactable.Table>
);
}
});
React.render(<TableComponent/>, document.getElementById('wrapper'));
</script>
</body>
</html> |
I'm gonna see what I can do about adding a Travis check to make sure build/ and lib/ are up-to-date 👀 |
Thanks for the failing case, too! will check it out |
Since react will not render child components until the parent component is rendered (exploratory hypotheis) We need to render the custom component first and then access the component. This uses a Component function of getData(). The getData function could/should be fast because after the render function all the data could be available via props or state I have not tested this with updating data from the custom component. Or if the custom component internally changes the data and they table needs to resort.
This reverts commit 1fc822d.
Use the structure returned by `react.createElement` to do a render of any child components with unrecognized types, and extract the data out of them if they themselves return a `<Tr>`
When instantiating child components in order to render them, also pass through the props given when creating the child component.
1f02dbd
to
3ca5bb7
Compare
if ([_tfoot.Tfoot, _thead.Thead, _tr.Tr].indexOf(child.type) >= 0) { | ||
reactableDescendant = child; | ||
} else { | ||
reactableDescendant = new child.type(child.props, child._context).render(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this works in react 0.14.
what's the status of this branch? Can it be merged to master? :) |
I wish I could help with this, just realised I need it as well. Can you guys do the merge? Pretty plz 😀 |
Could we just call it? Would be great to solve #156 |
Hello from the future! Would love for this to be included. |
Is the merge conflict resolved? I still have the same problem. |
Can this be merged? |
@revett I merged this and another patch in my fork https://github.com/bf/reactable |
^^^ signal boosting this, would love to see it fixed up and merged. |
This is the implementation side of the failing-test PR that @growlsworth originally submitted as #166