Fix tree line drawing by using a callback ref

In a non-dev build (no initial double-renders), the tree lines would not be
rendered correctly from the parent of a binop to its first child, because the
first child would be rendered before the parent, and the parent's ref hadn't
been set yet at that time. Switched from a normal ref to a callback-based ref
with explicit React state update to make sure that the child gets to know about
the parent's (later) rendered div element.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
This commit is contained in:
Julius Volz 2024-09-16 08:42:20 +02:00
parent 0180cf31aa
commit 50af7d589b

View file

@ -4,7 +4,6 @@ import {
useEffect, useEffect,
useLayoutEffect, useLayoutEffect,
useMemo, useMemo,
useRef,
useState, useState,
} from "react"; } from "react";
import ASTNode, { nodeType } from "../../promql/ast"; import ASTNode, { nodeType } from "../../promql/ast";
@ -60,7 +59,7 @@ const TreeNode: FC<{
node: ASTNode; node: ASTNode;
selectedNode: { id: string; node: ASTNode } | null; selectedNode: { id: string; node: ASTNode } | null;
setSelectedNode: (Node: { id: string; node: ASTNode } | null) => void; setSelectedNode: (Node: { id: string; node: ASTNode } | null) => void;
parentRef?: React.RefObject<HTMLDivElement>; parentEl?: HTMLDivElement | null;
reportNodeState?: (childIdx: number, state: NodeState) => void; reportNodeState?: (childIdx: number, state: NodeState) => void;
reverse: boolean; reverse: boolean;
// The index of this node in its parent's children. // The index of this node in its parent's children.
@ -69,13 +68,21 @@ const TreeNode: FC<{
node, node,
selectedNode, selectedNode,
setSelectedNode, setSelectedNode,
parentRef, parentEl,
reportNodeState, reportNodeState,
reverse, reverse,
childIdx, childIdx,
}) => { }) => {
const nodeID = useId(); const nodeID = useId();
const nodeRef = useRef<HTMLDivElement>(null);
// A normal ref won't work properly here because the ref's `current` property
// going from `null` to defined won't trigger a re-render of the child
// component, since it's not a React state update. So we manually have to
// create a state update using a callback ref. See also
// https://tkdodo.eu/blog/avoiding-use-effect-with-callback-refs
const [nodeEl, setNodeEl] = useState<HTMLDivElement | null>(null);
const nodeRef = useCallback((node: HTMLDivElement) => setNodeEl(node), []);
const [connectorStyle, setConnectorStyle] = useState<CSSProperties>({ const [connectorStyle, setConnectorStyle] = useState<CSSProperties>({
borderColor: borderColor:
"light-dark(var(--mantine-color-gray-4), var(--mantine-color-dark-3))", "light-dark(var(--mantine-color-gray-4), var(--mantine-color-dark-3))",
@ -97,10 +104,10 @@ const TreeNode: FC<{
// Select the node when it is mounted and it is the root of the tree. // Select the node when it is mounted and it is the root of the tree.
useEffect(() => { useEffect(() => {
if (parentRef === undefined) { if (parentEl === undefined) {
setSelectedNode({ id: nodeID, node: node }); setSelectedNode({ id: nodeID, node: node });
} }
}, [parentRef, setSelectedNode, nodeID, node]); }, [parentEl, setSelectedNode, nodeID, node]);
// Deselect node when node is unmounted. // Deselect node when node is unmounted.
useEffect(() => { useEffect(() => {
@ -173,16 +180,18 @@ const TreeNode: FC<{
// Update the size and position of tree connector lines based on the node's and its parent's position. // Update the size and position of tree connector lines based on the node's and its parent's position.
useLayoutEffect(() => { useLayoutEffect(() => {
if (parentRef === undefined) { if (parentEl === undefined) {
// We're the root node. // We're the root node.
return; return;
} }
if (parentRef.current === null || nodeRef.current === null) { if (parentEl === null || nodeEl === null) {
// Either of the two connected nodes hasn't been rendered yet.
return; return;
} }
const parentRect = parentRef.current.getBoundingClientRect();
const nodeRect = nodeRef.current.getBoundingClientRect(); const parentRect = parentEl.getBoundingClientRect();
const nodeRect = nodeEl.getBoundingClientRect();
if (reverse) { if (reverse) {
setConnectorStyle((prevStyle) => ({ setConnectorStyle((prevStyle) => ({
...prevStyle, ...prevStyle,
@ -202,7 +211,7 @@ const TreeNode: FC<{
borderTopLeftRadius: undefined, borderTopLeftRadius: undefined,
})); }));
} }
}, [parentRef, reverse, nodeRef, setConnectorStyle]); }, [parentEl, nodeEl, reverse, nodeRef, setConnectorStyle]);
// Update the node info state based on the query result. // Update the node info state based on the query result.
useEffect(() => { useEffect(() => {
@ -264,7 +273,7 @@ const TreeNode: FC<{
pos="relative" pos="relative"
align="center" align="center"
> >
{parentRef && ( {parentEl !== undefined && (
// Connector line between this node and its parent. // Connector line between this node and its parent.
<Box pos="absolute" display="inline-block" style={connectorStyle} /> <Box pos="absolute" display="inline-block" style={connectorStyle} />
)} )}
@ -391,7 +400,7 @@ const TreeNode: FC<{
node={children[0]} node={children[0]}
selectedNode={selectedNode} selectedNode={selectedNode}
setSelectedNode={setSelectedNode} setSelectedNode={setSelectedNode}
parentRef={nodeRef} parentEl={nodeEl}
reverse={true} reverse={true}
childIdx={0} childIdx={0}
reportNodeState={childReportNodeState} reportNodeState={childReportNodeState}
@ -403,7 +412,7 @@ const TreeNode: FC<{
node={children[1]} node={children[1]}
selectedNode={selectedNode} selectedNode={selectedNode}
setSelectedNode={setSelectedNode} setSelectedNode={setSelectedNode}
parentRef={nodeRef} parentEl={nodeEl}
reverse={false} reverse={false}
childIdx={1} childIdx={1}
reportNodeState={childReportNodeState} reportNodeState={childReportNodeState}
@ -422,7 +431,7 @@ const TreeNode: FC<{
node={child} node={child}
selectedNode={selectedNode} selectedNode={selectedNode}
setSelectedNode={setSelectedNode} setSelectedNode={setSelectedNode}
parentRef={nodeRef} parentEl={nodeEl}
reverse={false} reverse={false}
childIdx={idx} childIdx={idx}
reportNodeState={childReportNodeState} reportNodeState={childReportNodeState}