Allow specifying table rows in custom components #196

glittershark · 2015-09-30T05:03:54Z

This is the implementation side of the failing-test PR that @growlsworth originally submitted as #166

glittershark · 2015-09-30T05:07:15Z

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 componentDidMount() will never be called - and in fact its entire lifecycle will be completely discarded after the initial render of the Reactable.Table. That's far from perfect - it'd probably be possible to do the processing to pull the data out of the row or cell and then encode that as part of the __reactableMeta property that we're already saving on the individual components

trshafer · 2015-09-30T05:42:02Z

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 ;)

glittershark · 2015-10-02T01:35:24Z

for posterity, and as a note to myself, the commit that you commented on and then I force-pushed away: 4abe39c

glittershark · 2015-10-02T01:43:04Z

@growlsworth I think 1f02dbd should allow passing through child context, do you mind taking a look?

trshafer · 2015-10-02T23:29:08Z

In the parent, which holds the Table, this.context returns {router:…}, and in the child, which is the Tr, this.context still returns undefined.

trshafer · 2015-10-03T00:39:05Z

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.

trshafer · 2015-10-03T01:28:49Z

Ok. I got the test example to pass in my machine. FYI, the lib wasn't updated, which made requiring reactable not load the most up to date version. But I'm still having difficulty with react-router. Will see if I can make a breaking test outside my app.

trshafer · 2015-10-03T01:42:26Z

Ok I got it to fail. The issue is in the child's child's context. :(

Drop this bad boy in tmp/:

<!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>

glittershark · 2015-10-03T14:24:23Z

I'm gonna see what I can do about adding a Travis check to make sure build/ and lib/ are up-to-date 👀

glittershark · 2015-10-03T14:24:36Z

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.

trshafer · 2015-10-07T22:34:12Z

build/reactable.js

+                        if ([_tfoot.Tfoot, _thead.Thead, _tr.Tr].indexOf(child.type) >= 0) {
+                            reactableDescendant = child;
+                        } else {
+                            reactableDescendant = new child.type(child.props, child._context).render();


I don't think this works in react 0.14.

educastellano · 2015-12-14T13:56:49Z

what's the status of this branch? Can it be merged to master? :)

alfredoreduarte · 2016-01-30T23:53:48Z

I wish I could help with this, just realised I need it as well. Can you guys do the merge? Pretty plz 😀

singpolyma · 2016-03-08T22:19:36Z

that sub-component's componentDidMount() will never be called

Could we just call it? Would be great to solve #156

zacharyz · 2016-07-21T23:30:10Z

Hello from the future! Would love for this to be included.

paddotk · 2016-10-31T15:20:34Z

Is the merge conflict resolved? I still have the same problem.

revett · 2017-01-20T11:53:55Z

Can this be merged?

bf · 2017-01-26T21:38:33Z

@revett I merged this and another patch in my fork https://github.com/bf/reactable

kidmillions · 2017-05-12T19:58:33Z

^^^ signal boosting this, would love to see it fixed up and merged.

glittershark mentioned this pull request Sep 30, 2015

added failing test for rendering sub components #166

Closed

glittershark force-pushed the tr-custom-components branch 2 times, most recently from 4abe39c to 1eb6eb9 Compare October 2, 2015 01:21

trshafer and others added 5 commits October 3, 2015 10:27

Added failing test for rendering sub components

71b81bf

Revert "prototype solution to custom components"

3bd8e5a

This reverts commit 1fc822d.

Allow specifying table rows in custom components

7a937ca

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>`

Handle passing through context to child components

3ca5bb7

When instantiating child components in order to render them, also pass through the props given when creating the child component.

glittershark force-pushed the tr-custom-components branch from 1f02dbd to 3ca5bb7 Compare October 3, 2015 15:10

trshafer reviewed Oct 7, 2015
View reviewed changes

glittershark mentioned this pull request Mar 18, 2016

Unable to use other Tr-inherited components #256

Closed

nlaffey mentioned this pull request Sep 22, 2016

Customizing individual columns that are not in additional rows #102

Closed

glittershark mentioned this pull request Sep 29, 2016

The only possible children of <Table> are <Thead>, <Tr>, or one <Tfoot>. #320

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying table rows in custom components #196

Allow specifying table rows in custom components #196

glittershark commented Sep 30, 2015

glittershark commented Sep 30, 2015

trshafer commented Sep 30, 2015

glittershark commented Oct 2, 2015

glittershark commented Oct 2, 2015

trshafer commented Oct 2, 2015

trshafer commented Oct 3, 2015

trshafer commented Oct 3, 2015

trshafer commented Oct 3, 2015

glittershark commented Oct 3, 2015

glittershark commented Oct 3, 2015

trshafer Oct 7, 2015

educastellano commented Dec 14, 2015

alfredoreduarte commented Jan 30, 2016

singpolyma commented Mar 8, 2016

zacharyz commented Jul 21, 2016

paddotk commented Oct 31, 2016

revett commented Jan 20, 2017

bf commented Jan 26, 2017

kidmillions commented May 12, 2017

Allow specifying table rows in custom components #196

Are you sure you want to change the base?

Allow specifying table rows in custom components #196

Conversation

glittershark commented Sep 30, 2015

glittershark commented Sep 30, 2015

trshafer commented Sep 30, 2015

glittershark commented Oct 2, 2015

glittershark commented Oct 2, 2015

trshafer commented Oct 2, 2015

trshafer commented Oct 3, 2015

trshafer commented Oct 3, 2015

trshafer commented Oct 3, 2015

glittershark commented Oct 3, 2015

glittershark commented Oct 3, 2015

trshafer Oct 7, 2015

Choose a reason for hiding this comment

educastellano commented Dec 14, 2015

alfredoreduarte commented Jan 30, 2016

singpolyma commented Mar 8, 2016

zacharyz commented Jul 21, 2016

paddotk commented Oct 31, 2016

revett commented Jan 20, 2017

bf commented Jan 26, 2017

kidmillions commented May 12, 2017