roderickhsiao / react-in-viewport

Detect if React component is in viewport
https://roderickhsiao.github.io/react-in-viewport
MIT License
350 stars 30 forks source link

inViewport returns wrong result intermittently in case of large number of items #81

Closed sjrules closed 2 years ago

sjrules commented 3 years ago

I am using Material tree , and rendering label in Tree based on whether tree item is in viewport or not . But 1 out of 5 times , inViewport is returning false or null , because of which label rendering fails even though tree item is in view port .

This is observed intermittently but can easily be reproduced .

roderickhsiao commented 3 years ago

Thanks for reporting, if you could provide minimal setup/snippets will better help to debug the issue

sjrules commented 3 years ago

Intermittent Issue is seen after scrolling for a while , or after expanding collapsing node in case of large data is large say Tree having 10k nodes . Also it messes up the scroll ..

Please find below code ,

import React, { useEffect, useState } from 'react';
import { makeStyles } from '@material-ui/core/styles';
import TreeView from '@material-ui/lab/TreeView';
import ExpandMoreIcon from '@material-ui/icons/ExpandMore';
import ChevronRightIcon from '@material-ui/icons/ChevronRight';
import TreeItem from '@material-ui/lab/TreeItem';
import handleViewport from 'react-in-viewport';
type Model  = {
    text : string; // unique id for nodes 
    value : string; // value to display
    children ?: Model[];
}
const useStyles = makeStyles({
    root: {
        height: 110,
        flexGrow: 1,
        maxWidth: 400,
    },
});
export default function RecursiveTreeView(props: any) {
    const classes = useStyles();
    const [allParentIds, setAllParentIds] = useState<string[]>([]);
    useEffect(() => {
        const ids: string[] = [];
        populateAllParentNodeIds(ids, props.data);
        setAllParentIds(ids);
    }, []);
    const [expanded, setExpanded] = useState<string[]>([props.data.text]);
    const populateAllParentNodeIds = (ids: string[], node: Model) => {
        if (node.children) ids.push(node.text);
        node.children?.forEach((x) => {
            populateAllParentNodeIds(ids, x);
        });
    };
    const renderTree = (nodes: Model) => {
        return (
            <ViewportTree
                nodes={nodes}>
                {Array.isArray(nodes.children) ? nodes.children.map((node) => renderTree(node)) : null}
            </ViewportTree>
        );
    }
    const handleToggle = (_e: any, nodeIds: string[]) => {
        setExpanded(nodeIds);
      };
    return (
        <div style = {{display : 'flex'}}>
             <button onClick = {() => setExpanded(allParentIds)} >Expand All</button>
            <TreeView
                className={classes.root}
                defaultCollapseIcon={<ExpandMoreIcon />}
                defaultExpanded={['root']}
                defaultExpandIcon={<ChevronRightIcon />}
                expanded={expanded}
                onNodeToggle={handleToggle}
            >
                {renderTree(props.data)}
            </TreeView>           
        </div>   );}
function MatTreeItem(props: any) {
    const { inViewport, forwardedRef, nodes } = props;
    const renderLabel = (node: Model) => {
        return (
            <div >
                {/* {inViewport  && <span>In view</span>} */}
                {node.value}
            </div> )   }
    return (
        <TreeItem
            ref={forwardedRef}
            key={nodes.text} nodeId={nodes.text} label={inViewport ? renderLabel(nodes) : <></>}
            {...props}>
        </TreeItem>
    )}
const ViewportTree = handleViewport(MatTreeItem, /** options: {}, config: {} **/);

Versions -

    "react": "^17.0.2",
    "react-dom": "^17.0.2",
    "react-in-viewport": "^1.0.0-alpha.16",
     "@material-ui/core": "latest",
    "@material-ui/icons": "latest",
    "@material-ui/lab": "latest"
roderickhsiao commented 3 years ago

I wouldn't suggest you use intersection-observer for a large list on each item. Browser API is not without cost, observing large data on scroll is very expensive (thats why you are seeing scroll behaviors is janky). Your whole tree is re-rendering when its in viewport on scroll, the library could report value after CPU idle, which already pass the scroll position.

Virtualizing list needs much more detail memoization.

If you just want to enhance render performance, I'll just suggest to use https://web.dev/content-visibility/

This is more a render performance issuse