mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.11k stars 32.34k forks source link

[docs] Table virtualized #7450

Closed damianobarbati closed 5 years ago

damianobarbati commented 7 years ago

@oliviertassinari would you consider to add a virtual container to the tbody to increase performance when rendering long lists without pagination? I'll take a stab at this if you could just point me in the right direction (react-virtualized?)

oliviertassinari commented 7 years ago

We hope that the current API allow simple integration with react-virtualized. We are open to do any change to the implementation in order to make the integration simpler.

oliviertassinari commented 7 years ago

Having an demo example in the docs would be awesome.

oliviertassinari commented 7 years ago

If that can help, here is an integration example with the List/Menu https://codesandbox.io/s/oQ0nkl5pk.

damianobarbati commented 7 years ago

@oliviertassinari I made it without react-virtualized because it was a mess and it didn't play nice with mui components. Furthermore, my current solution is a pure-css fixed thead and scrollable tbody with dynamic table height and dynamic columns width (tested on Safari as well). The always-on-top thead with a scrollable tbody is a must for data grids.

I'm adding scroll debouncing, tweaking performance and fixing some glitch but it's working great: I'm "virtually" rendering about ~2400rows, sooo smooth.

out

Questions on Table components:

  1. how to use ref with MUI components? I'm auto-computing TableThead height: problem is that

    <TableHead ref={ ref => this.tableThead = ref }>

    is not returning the actual DOM container element of TableThead but the React instance (native behaviour of React when applying ref on a not-DOM element) because that's something that should be applied in the TableThead component itself; other components may need this, so why not having a refHandler on every MUI component which applies the given callback in the first rendered element? (I hope I managed to explain this out @_@) My current solution is a weird

    const theadHeight = ReactDOM.findDOMNode(this.tableThead).clientHeight; // eslint-disable-line react/no-find-dom-node

    which means using the deprecated findDomNode and disabling eslint as well.

  2. why is border-bottom applied on cells instead of rows?

  3. what about having a stripe={true} prop to automatically stripe TableBody rows?

  4. why does .MuiIconButton.root have a zIndex style property? πŸ€”

I hope to publish something stable about this soon so you can give a look it that's something which can be adopted in a general way.

Sorry to bother this much but I'm heavily using mui@next so I'm spotting a lot of use-cases!

oliviertassinari commented 7 years ago

I made it without react-virtualized because it was a mess and it didn't play nice with mui components.

Oh, that's a shame on us. Any idea what we could do to make is possible?

  1. You can try the innerRef to get a ref on the TableHead. ref will get you a ref on the withStyles(TableHead) component. If that doesn't work we could expose a rootRef property. That's a pattern we are already using.

  2. No clue

  3. What do you mean by stripe?

  4. I have removed it in #7467

damianobarbati commented 7 years ago
Oh, that's a shame on us. Any idea what we could do to make is possible?

Actually is a mix of react-virtualized own containers, native table/thead/tbody DOM elements and the need of having a scrollable tbody which caused the problem. Too much work was needed with it: simply making an ad-hoc solution for MuiTable was a breeze.

  1. exposing a rootRef would be great (assuming TableThead main element in the render method is the Thead DOM element itself)

  2. should border-bottom be moved to tr then? Should I move this into a dedicated issue?

  3. I mean striped rows: easy to achieve with like tr:nth-child(odd) { backgroundColor: f2f2f2 but since we already have the handy hover={true} coloring why not having the an handy striped={true}? :)

  4. great!

oliviertassinari commented 7 years ago
  1. πŸ‘ Sounds like a plan, here is an example, a PR is welcomed.
  2. Bootstrap is doing the same, I think that it's providing more flexibility by using it on the TableCell.
  3. Because it's not in the specification and it can be simply implemented on userland?
  4. πŸ‘
damianobarbati commented 7 years ago
  1. I'm not sure about simply having ref={rootRef} on every component. Devs who are using it are probably (mostly) in need to have control over the DOM. In your example <FormControl ref={rootRef} ...> will again return a reference to the React component (because FormControl is not a DOM element) and will have the developer use the workaround I had to use. Maybe we should have the rootRef behave like the following:

Is the container of the rendered component a DOM element?

The catch-all solution I can think about is having muiRef and rootRef properties where the former applies the callback to the "mui component" container (if any) and the latter applies the callback as I said before. This way Developer can easily have both/either MUI ref and DOM ref (whatever he needs, whenever he needs). Do you think this make sense?

  1. really? @_@

  2. ops! I always forget about the material specs

oliviertassinari commented 7 years ago
  1. Oh, I haven't thought about that case. The issue you are referring to is the following:
    • Component A takes a ref and a rootRef property
    • React intercept ref and apply it on the element
    • Component B receive a rootRef property and apply it as a ref on the root Component C
    • React intercept this ref and apply it on the element

But what if Component C also have a rootRef property? Component A can be on user-land, Component B can be the TextField and Component C can be the FormControl.

So my answer, at first we introduced rootRef as a simple helper. We haven't though about industrialization. So, I think that the current behavior is great as deterministic, simple and without the limitation you are describing, if people are interested in accessing the underlying dom element, they can use the findDOMNode API.

kgregory commented 6 years ago

@damianobarbati Was react-virtual-list the experiment with Table and virtualization mentioned here?

I'm guessing listComponent = TableBody, itemComponent = TableRow?

techniq commented 6 years ago

I've started this project that uses react-virtualized with Material 1.0 you might be interested in.

https://github.com/techniq/mui-table

It's still a work in progress (see https://github.com/techniq/mui-table/blob/master/TODO.md) but might be useful to you. Docs are lacking but take a look at the stories to get a feel for how to use it.

oliviertassinari commented 6 years ago

@techniq This looks promising πŸ˜„ ! I think it would be great to revamp the table documentation section at some point to have: - https://github.com/DevExpress/devextreme-reactive (commercial license)

We can also add it to the https://material-ui.com/discover-more/related-projects/#material-ui-specific-projects section. So, let us know when the project is more mature. I think that we can accept a pull request to promote the project :).

eyn commented 6 years ago

I have done a PR for react-virtual-list here https://github.com/clauderic/react-tiny-virtual-list/pull/30 which allows its usage with material-ui. I struggled to get react-virtual to work and create valid HTML as it has div's hard coded.

cc: @kgregory

kgregory commented 6 years ago

@eyn nice work! I just finished up an implementation of an infinite-scrolling table using react-virtualized and material-ui. I hope to throw a gist together soon. When I do I will link it here for any future visitors of this issue and possibly as an example in the docs.

Neofox commented 6 years ago

@kgregory did you finished your gist? I'm still looking for a basic solution for virtualized Material UI table and it's not easy to find... It would be a great help!

Thanks for your work!

kgregory commented 6 years ago

@neofox No, sorry. I never got around to it, but your reply is a nice reminder. I will try to find time to put something together.

rolandjitsu commented 6 years ago

@oliviertassinari your example in one of the first comments that supposedly shows how to use the material list with the react virtualized list is not working. Do you still have that example laying around?

oliviertassinari commented 6 years ago

@rolandjitsu Sorry, I don't any examples. It's what this issue is about :).

rolandjitsu commented 6 years ago

@oliviertassinari got it. I thought this issue was about the table and not the <List>, or it also covers the lists?

oliviertassinari commented 6 years ago

@rolandjitsu The list, the table, the paper, the grid list. It's all the same. This issue is about adding a simple demo so people can see the pattern and apply it to different components. The table is a component you likely gonna need virtualization.

hassan-zaheer commented 6 years ago

@oliviertassinari I would like to give this a crack, if you can point me in the right direction. As far as I can tell, we need a basic gist with material-ui@next integrated with react-virtualized?

oliviertassinari commented 6 years ago

@hassan-zaheer Yes, I think that a simple demo with react-virtualized at the end of the tables docs page will do the trick :).

zd-project commented 6 years ago

Hi. Happen to stumble upon this thread when researching for my project. I would like to know if there's any update here concerning the demo or usage documentation? @kgregory Or can you provide any insight in incorporating the two? It'd be nice to make the table look coherent with style

techniq commented 6 years ago

@zd-project You can take a look at my mui-virtualized-table project.

It used to be mui-table but I decided to fork the 2 and support different use cases (mui-table leverages the DOM directly and lets you use things like position: sticky and <table> layout (row/col spans, auto widths, etc) that were difficult with react-virtualized.

With that said, mui-virtualized-table just added column resizing from a nice PR.

rodoabad commented 6 years ago

Where are we on this guys?

cloudlena commented 6 years ago

@techniq, thanks for mui-virtualized-table! It's great! Is there any chance we could get it merged into mui-org/material-ui?

eps1lon commented 6 years ago

I'd be happy if somebody would open a PR that introduces a demo to the docs. This way we could examine if we can improve the table API for better integration with virtualization strategies.

@mastertinner Why do we need to add this to the core? Virtualization is an optimization strategy. Not everybody renders large data sets so I would not burden everybody's bundle with it.

oliviertassinari commented 6 years ago

@mastertinner We could start by referencing the project it in the documentation, for instance here: https://material-ui.com/demos/tables/#advanced-use-cases :)

cloudlena commented 6 years ago

πŸ‘ sounds like a plan

techniq commented 6 years ago

@mastertinner While I still use it (when virtualization is needed), I also use my newer mui-table when I don't need a virtualization, and can leverage the dom (using table, tr, td, or css grid for layout, removing the need for explicit column widths, etc) and enables some advanced patterns like multiple position: sticky headers (like this)

IssueHuntBot commented 6 years ago

@rogerstorm funded this issue with $30. See it on IssueHunt

joshwooding commented 6 years ago

Decided to play around with this today, I do not recommend 😭 Here's what I've produced so far: virtualize

Please excuse the lag, my laptop isn't great πŸ‘Ž It uses Material-UI Components and react-virtualized components

worudso commented 6 years ago

@joshwooding I've done exact same thing what you've done, virtualized + fixed header(with scrollbar on body only). If your implementation uses two seperate <Table>s, then I think you got the same problems I've found.

Well, the point is a component inside <TableCell> is inevitable. I wonder if you solved this problem in a elegant way. If not, I think virtualized + fixed header(with scrollbar on body only) is hard to be a small(as small as its function),well defined component.

joshwooding commented 6 years ago

@worudso Currently it uses only one <Table> I need to find a way to pass numeric down but I might be able to get that working. It’s definitely not a small Component and my code’s still a bit all over the place but I’m going to work on it some more and see if I can refine it

kgregory commented 6 years ago

I've created a sandbox that demonstrates using material-ui and react-virtualized to produce a virtualized table. This particular example uses react-virtualized to size the Table to the height of the window.

The MuiVirtualizedTable component from the example is also available in this gist.

MuiVirtualizedTable can accept any prop supported by a Table. The rowClassName is not directly applied, but is instead applied in addition to other styles defined in the MuiVirtualizedTable.

The columns prop expects an array of objects that is used when rendering cells for each column. Each object can accept any prop supported by Column.

My apologies for taking so long. I've been busy.

Janpot commented 6 years ago

FYI, there's now also react-window which is supposed to be react-virtualized successor. In case anyone wanted to try that one out.

oliviertassinari commented 6 years ago

@kgregory Thank you for sharing the codesandbox.

@joshwooding Great demo. It's great to have a fixed header :). I hope you will be able to complete the implementation! Regarding the numeric forwarding, what's preventing you from applying directly on the cells? react-window vs react-virtualized

joshwooding commented 6 years ago

@oliviertassinari I've set myself the goal of using the Table component's API and right now it doesn't look like you can pass custom props via the Column component. There are probably ways to do it without using the react-virtualized way though

oliviertassinari commented 6 years ago

@joshwooding Did you try https://github.com/bvaughn/react-virtualized/blob/master/docs/Table.md#headerrowrenderer?

joshwooding commented 6 years ago

@oliviertassinari I've tried using HeaderRowRenderer but it looks like you can't pass custom props through it without manipulating the existing props since this is what is called to create the columns to pass through to the headerRenderer:

const renderedHeader = headerRenderer({ columnData, dataKey, disableSort, label, sortBy, sortDirection, });

joshwooding commented 6 years ago

@oliviertassinari For a fixed height table you can just edit @kgregory's demo slightly: https://codesandbox.io/s/6vo8y0929k

oliviertassinari commented 6 years ago

@joshwooding Looks great! I can't think of anything missing to make it an official demo.

IssueHuntBot commented 6 years ago

@issuehuntfest has funded $60.00 to this issue. See it on IssueHunt

joshwooding commented 6 years ago

@oliviertassinari I'll work on implementing this :)

IssueHuntBot commented 5 years ago

@joshwooding has submitted a pull request. See it on IssueHunt

IssueHuntBot commented 5 years ago

@oliviertassinari has rewarded $81.00 to @joshwooding. See it on IssueHunt

kgregory commented 5 years ago

@oliviertassinari has rewarded $81.00 to @joshwooding. See it on IssueHunt

Well deserved. Nice work @joshwooding!

aguywithcode commented 5 years ago

I have a similar need but for the List component...would the same approach work for making the list work?

oliviertassinari commented 5 years ago

@mdizzy Yes, we could add a similar demo for the list.

MastroLindus commented 5 years ago

I am trying right now to make a list virtual with react-window (as the creator suggested to use it in place of virtualized if you don't need the full API, but in the end the API for a list doesn't really change that much)

So far I have two big issues: 1)List items style is broken, especially for stuff like right icons/actions. I guess that the the virtual list injected style doesn't play nicely with the styling from material-ui 2) Sticky subheaders get really messy as well, probably for the same reason (react-window use position absolute to layout the items)

I am going to probably try react-virtualized to see if they work better together