clauderic / react-tiny-virtual-list

A tiny but mighty 3kb list virtualization library, with zero dependencies 💪 Supports variable heights/widths, sticky items, scrolling to index, and more!
https://clauderic.github.io/react-tiny-virtual-list/
MIT License
2.46k stars 166 forks source link

Allow user to render their own wrapper (useful for tables) #30

Closed eyn closed 6 years ago

eyn commented 6 years ago

Thanks for the great library! I've added a renderWrapper function this allows a user to use components other the <div> for the two wrappers (such as table and tbody with appropriate styling).

An example using material-ui:

import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { Table, TableHead, TableBody, TableCell, TableRow } from 'material-ui';
import { withStyles } from 'material-ui/styles';

import VirtualList from '../../src/';
import './demo.css';

const styles = ({
  table: {
    display: 'block',
    '& thead': {
      display: 'block',
    },
    '& tbody': {
      display: 'block',
    },
    '& tr': {
      display: 'table',
      width: '100%',
    },
  },
});

const TableWithRef = withStyles<any>(styles)(
  ({ classes, className, tableRef, children, ...others }: any) =>
    <table className={`${classes.table} ${className}`} ref={tableRef} {...others}>
      {children}
    </table>);

class Demo extends React.Component {
  renderWrapper = (outerProps, ref, innerStyle, items) => {
    return (
      <Table tableRef={ref} {...outerProps} component={TableWithRef}>
        <TableHead>
          <TableRow>
            <TableCell>Test</TableCell>
          </TableRow>
        </TableHead>
        <TableBody style={innerStyle}>
          {items}
        </TableBody>
      </Table>);
  }

  renderItem = ({ style, index }: { style: React.CSSProperties, index: number }) => {
    return (
      <TableRow className="Row" style={style} key={index}>
        <TableCell>
          Row #{index}
        </TableCell>
      </TableRow>
    );
  }

  render() {
    return (
      <div className="Root">
        <VirtualList
          width="auto"
          height={400}
          itemCount={1000}
          renderItem={this.renderItem}
          renderWrapper={this.renderWrapper}
          itemSize={58}
          className="VirtualList"
        />
      </div>
    );
  }
}

ReactDOM.render(<Demo />, document.querySelector('#app'));
codecov[bot] commented 6 years ago

Codecov Report

Merging #30 into master will increase coverage by 0.1%. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #30     +/-   ##
========================================
+ Coverage    93.6%   93.7%   +0.1%     
========================================
  Files           3       3             
  Lines         125     127      +2     
  Branches       14      14             
========================================
+ Hits          117     119      +2     
  Misses          8       8
Impacted Files Coverage Δ
src/index.tsx 100% <ø> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e97d432...3ed4012. Read the comment docs.

eyn commented 6 years ago

Hi @clauderic - any chance of getting some feedback/getting this merged? Thanks for the awesome library :)

thorjarhun commented 6 years ago

Hi @eyn. I really like the functionality added in this PR! Thank you so much for sharing!

I'd like to use the wrapper from your example using material-ui but I'm using the material-ui v1 which doesn't have support for the old fixedHeader prop. I'm also using the scrollToTransition proposal (open at time of writing) but found that I needed to modify the getOffsetForIndex method to add the height of the header to the value returned. I would love a fixed header and I think some solutions to achieve that will resolve my problem with getOffsetForIndex. I've done some research and it appears that there are many solutions for achieving a fixed-header table but I haven't yet found one that works. By chance, are you using a material-ui table with a fixed header or have advice on what I should try to achieve it? Perhaps we could even add your example as a suggestion for using renderWrapper in the docs, assuming this PR goes through!

eyn commented 6 years ago

Hi @thorjarhun

Glad you found the PR useful! I've also struggled with getting a fixed header and a better solution is on my to-do list. At the moment I've gone down the route of using two tables - one for the header and one for the body with fixed column % widths as I know the length of most of the data I'm displaying.

Would that work for you?

thorjarhun commented 6 years ago

That would be fantastic! Is the code hosted somewhere for reference? I couldn't find anything that looks like a to-do list in your repo's.

eyn commented 6 years ago

Ahh I don't have any public code for that - I've just copied the bit out of the relevant file below:

          <div className={classes.tableContainer}>
            <Table className={classes.headerTable}>
              <TableHead>
                <TableRow>
                  <TableCell
                    className={classes.checkboxCell}
                    padding="checkbox"
                  >
                    <Checkbox
                      indeterminate={
                        select.selectedRowCount > 0 &&
                        select.selectedRowCount < rows.length
                      }
                      checked={select.selectedRowCount === rows.length}
                      onClick={this.handleSelectAllClick}
                    />
                  </TableCell>
                  {columns.map(col => (
                    <TableHeaderCell
                      key={col.name}
                      col={col}
                      sort={sort}
                      showFilter={showFilterList || col.filter.isFiltered}
                    />
                  ))}
                  <TableCell padding="none" className={classes.cellToolbar} />
                </TableRow>
              </TableHead>
            </Table>
            <VirtualList
              width="auto"
              height={height}
              itemCount={rows.length}
              renderItem={this.renderItem}
              renderWrapper={this.renderWrapper}
              tableState={tableState}
              itemSize={48}
              overscanCount={20}
              className={classes.bodyTable}
              rows={rows}
            />
          </div>

Hopefully that will get you pointed in the right direction

mihiramin89 commented 6 years ago

Any updates on when this will be merged? I am using a table that can really use this for the body.

prescottprue commented 6 years ago

I agree this would be useful. Any input on when it is getting merged?

vste23 commented 6 years ago

Hi. Very useful PR that would be awesome to see merged into this library. Will it be merged?

towfiqi commented 6 years ago

+1 Kindly merge this..

atrauzzi commented 6 years ago

Yikes, what's the holdup here?

baronmog commented 6 years ago

Is this going anywhere? I'm working on a project that already uses react-tiny-virtual-list. We're in the process of integrating material-ui into it and would like to continue using react-tiny-virtual-list, but...

eyn commented 6 years ago

I've tried contacting @clauderic via other channels but no success so doesn't look like it 😢 I have been using this patch in production for quite a while so it does work..

clauderic commented 6 years ago

This PR has merge conflicts and I'm not 100% satisfied with the approach, the function signature for the renderWrapper is too complex in my opinion.

I'm going to close this PR for the moment as I don't have any intention of merging it in it's current state, but we can keep the discussion open.

To be clear, I agree that this is an important use case and I am willing to support it, but I'd rather spend a bit more time to make the API more elegant.