-
-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow grouped table #792
base: master
Are you sure you want to change the base?
Conversation
This looks about right, but I have not run it locally. Small note - can you use yarn (v1), so that you don't commit the package-lock.json? |
Alright I will try to get around to that today! Would you prefer to extract it to a separate component like GroupedVirtuoso? |
Good question - I guess yes, since the two modes are not fully compatible. |
Could you give me more infos on what is not compatible? |
The flat component accepts total count and data. The grouped one accepts groupCounts (grouped data mode is to be done, eventually). Check this: https://github.com/petyosi/react-virtuoso/blob/master/src/components.tsx#L257-L273 |
Thanks! |
Does this look good to you? |
Sorry, I will have to get back to that over the weekend. Quite a lot going on. |
No worries, I noticed my implementation makes it so that the last item may be cutoff. It seems like I would need to add height to the viewport when you have a stickied group. Do you have an idea on the best way to implement this. |
This might be a matter of reporting additional height into the internal system. |
I was wrong actually in my actual project I had a missing forward ref on a HeaderComponent, the problem is actually the inverse, there is too much height on the bottom, I'm guessing it's because the stickied group is not substracted from the totalHeight. |
I fixed the conflicts and followed the new file structures for the interfaces |
@maloguertin @petyosi can you add option to TableVirtuoso to have context props too? I want to pass TableBody any state variable but I don't have an option without context |
Hey all, I am currently using |
I think there was something unresolved in the PR?
|
I haven't had time to look into this for a while. I have managed to mostly make it work by setting a negative margin to compensate for the height inequality but it's more of a hacky solution. |
What the status of this PR? Is it too outdated and will be difficult to merge? What need to fix to be able to merge it and support grouped table. |
hello @maloguertin , Will this feature be made available anytime soon:) ? |
Hi this is probably more of a WIP but I wanted to get your pulse on that. We needed to have sticky group headers in our table and I saw that not much was missing from the table implementation for it to work.
I added an example (group-table.tx)