Skip to content
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

Fix: Use forwardRef for scrollable virtualized tree #8186

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
use forwardRef for scrollable virtualized tree
  • Loading branch information
Michael Büßemeyer authored and Michael Büßemeyer committed Nov 12, 2024
commit fec41221a04dea4252e9f5dcfe4420aaacfaac5c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Tree as AntdTree, type TreeProps } from "antd";
import type { BasicDataNode } from "antd/es/tree";
import { throttle } from "lodash";
import { useCallback, useRef } from "react";
import { forwardRef, useCallback, useRef } from "react";
import type RcTree from "rc-tree";

const MIN_SCROLL_SPEED = 30;
@@ -10,8 +10,10 @@ const MIN_SCROLL_AREA_HEIGHT = 60;
const SCROLL_AREA_RATIO = 10; // 1/10th of the container height
const THROTTLE_TIME = 25;

function ScrollableVirtualizedTree<T extends BasicDataNode>(
props: TreeProps<T> & { ref: React.RefObject<RcTree> },
// React.forwardRef does not support generic types, so we need to define the type of the ref separately.
function ScrollableVirtualizedTreeInner<T extends BasicDataNode>(
props: TreeProps<T>,
ref: React.Ref<RcTree>,
) {
const wrapperRef = useRef<HTMLDivElement>(null);
// biome-ignore lint/correctness/useExhaustiveDependencies: biome is not smart enough to notice that the function needs to be re-created when wrapperRef changes.
@@ -56,9 +58,15 @@ function ScrollableVirtualizedTree<T extends BasicDataNode>(

return (
<div ref={wrapperRef}>
<AntdTree {...props} onDragOver={onDragOver} />
<AntdTree {...props} onDragOver={onDragOver} ref={ref} />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref is now a separate arg and thus needs to be passed explicitly 🤷‍♂️

</div>
);
}

const ScrollableVirtualizedTree = forwardRef(ScrollableVirtualizedTreeInner) as <
T extends BasicDataNode,
>(
props: TreeProps<T> & { ref?: React.Ref<RcTree> },
) => ReturnType<typeof ScrollableVirtualizedTreeInner>;

Comment on lines +66 to +71
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way of using forwardRef was suggested by AI overlord as forwardRef does not seem to support generic typing

export default ScrollableVirtualizedTree;
Original file line number Diff line number Diff line change
@@ -1904,7 +1904,7 @@ class SegmentsView extends React.Component<Props, State> {
overflow: "hidden",
}}
>
<ScrollableVirtualizedTree<SegmentHierarchyNode>
<ScrollableVirtualizedTree
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can now be removed as the type casting is results in a "better" / "more understandable" typing to ts -> Explicitly telling the generic type is not needed anymore.

allowDrop={this.allowDrop}
onDrop={this.onDrop}
onSelect={this.onSelectTreeItem}