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
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fix that scrolling in the trees and segments tab did not work while dragging. [#8162](https://github.com/scalableminds/webknossos/pull/8162)
- Fixed that uploading a dataset which needs a conversion failed when the angstrom unit was configured for the conversion. [#8173](https://github.com/scalableminds/webknossos/pull/8173)
- Fixed that the skeleton search did not automatically expand groups that contained the selected tree [#8129](https://github.com/scalableminds/webknossos/pull/8129)
- Fixed interactions in the trees and segments tab like the search due to a bug introduced by [#8162](https://github.com/scalableminds/webknossos/pull/8162). [#8186](https://github.com/scalableminds/webknossos/pull/8186)
- Fixed a bug that zarr streaming version 3 returned the shape of mag (1, 1, 1) / the finest mag for all mags. [#8116](https://github.com/scalableminds/webknossos/pull/8116)
- Fixed sorting of mags in outbound zarr streaming. [#8125](https://github.com/scalableminds/webknossos/pull/8125)
- Fixed a bug where you could not create annotations for public datasets of other organizations. [#8107](https://github.com/scalableminds/webknossos/pull/8107)
Expand Down
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;
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Up @@ -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}
Expand Down