added failing test for rendering sub components #166
Conversation
trshafer
force-pushed
the
tr-custom-components
branch
from
144a612
to
b8a5c5e
Compare
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.
|
Hey @glittershark, I did a spike on a proposed solution. Overall I give it a 3/5 stars of a solution. It requires a double render which isn't great. Also, I haven't tested it in sorting/filtering/updating which is the main reason of using reactable. I don't think this solution addresses @alfredo-cp's issue #159. Let me know what you think. |
|
👍 👍 👍 if every FOSS contributor were like you, the world would be a better and happier place |
|
Overall this is a pretty solid PR. Tests are good, guinea pig is good. |
|
Thanks. This was way more into the internals of React than I've ever gone. It may be possible to access sub component's children later in the component lifecycle and avoid the |
|
Hey @glittershark, I feel like we left this conversation at an ambiguous state. Would you like me to continue down an implementation path, or would rather take the reins from here? |
|
@growlsworth @glittershark What approach should I take starting from scratch? These are 2 components table & row. |
|
I got the same errors from #159 before starting over |
|
We've had to build our table from scratch too ... I will get our developer to post our finished table as an idea of things that we needed to be able to do. |
|
thanks would really appreciate it @aidanlister |
|
@growlsworth I'm pretty swamped with my day job as of late, feel free to take a crack at doing this with |
|
Will do. Other things are prioritized for me right now, but I'll get around to completing this. |
|
@growlsworth You would be a savior man reactable does exactly what I need for this project |
|
Can you check my code out @growlsworth |
|
Hey @glittershark , just wanted to say this is still on my radar. Priorities got shifted around at work, as they always do, so I haven't had time to put into this. |
|
@growlsworth I've got some time and would love to take a crack at this. Made any progress yet? |
|
I have made no progress. It would be great if you could put your brain on On Sat, Sep 19, 2015, 2:08 PM Griffin Smith [email protected]
|
|
👍 |
|
@growlsworth came up with the exact solution to this at 1am immediately before falling asleep, like with all good tough programming problems. That's in #196, please take a look and let me know what you think. Gonna close this PR since all of its commits are over there in one form or another |
Hey @glittershark,
I was looking at your other bugs and you mentioned that you like PRs with failing tests. So here's a failing test that addresses #156.
It would be awesome if you could make better headway than I did. I would also learn a little more on how React works. I got stuck when parsing the children at https://github.com/glittershark/reactable/blob/master/src/reactable/table.jsx#L64, because the sub components don't have any children.
Here's a picture of a guinea pig wearing a tiny sombrero to inspire you: