From 8d15018986fdc8159fec1b679eda343543d5ba2d Mon Sep 17 00:00:00 2001 From: Thomas Nordquist Date: Wed, 26 Jun 2019 12:56:31 +0200 Subject: [PATCH] Refactor TreeNodeComponent --- app/src/actions/visibleTreeTraversal.ts | 103 +++++++++--------- app/src/components/Tree/TreeNode.tsx | 93 +++++++++------- .../Tree/useViewModelSubscriptions.tsx | 2 +- 3 files changed, 111 insertions(+), 87 deletions(-) diff --git a/app/src/actions/visibleTreeTraversal.ts b/app/src/actions/visibleTreeTraversal.ts index 8019964..446ec1d 100644 --- a/app/src/actions/visibleTreeTraversal.ts +++ b/app/src/actions/visibleTreeTraversal.ts @@ -63,52 +63,15 @@ function nextVisibleElementInTree( node: q.TreeNode, direction: 'next' | 'previous' ): q.TreeNode | undefined { - const startNode = (node.sourceEdge && node.sourceEdge.source) || tree - const nodes = flattenNeighbors(settings, node, startNode) - const idx = nodes.findIndex(n => n.path() === node.path()) - const indexDirection = direction === 'next' ? 1 : -1 - return nodes[idx + indexDirection] -} - -/** Used to select partial relevant trees, to prevent the whole tree from being flattened */ -function flattenNeighbors( - settings: SettingsState, - selected: q.TreeNode, - treeNode: q.TreeNode -): Array> { - let candidates: Array> = [] - const nextNode = findNextNodeDownward(settings, selected) - - const neighborsOfSelected = sortedNodes(settings, treeNode) - const nodeIdx = neighborsOfSelected.findIndex(n => n.path() === selected.path()) - const previousNeighbor = neighborsOfSelected[nodeIdx - 1] - const parentNode = selected.sourceEdge && selected.sourceEdge.source - - if (previousNeighbor) { - candidates = candidates - .concat(flattenVisibleTree(settings, previousNeighbor)) - .concat(flattenVisibleTree(settings, selected)) - } else if (parentNode) { - candidates = candidates.concat(flattenVisibleTree(settings, parentNode)) + if (direction === 'next') { + return findNextNodeDownward(settings, node) + } else { + return findNextNodeUpward(settings, node) } - - return nextNode ? candidates.concat([nextNode]) : candidates } /** Not very efficient but easy to implement, complexity should not be an issue here */ -function flattenVisibleTree( - settings: SettingsState, - treeNode: q.TreeNode -): Array> { - return [treeNode].concat( - sortedNodes(settings, treeNode) - .filter(isTreeNodeVisible) - .map(node => flattenVisibleTree(settings, node)) - .reduce((a, b) => a.concat(b), []) - ) -} - -function findNextNodeDownward( +function findNextNodeUpward( settings: SettingsState, treeNode: q.TreeNode ): q.TreeNode | undefined { @@ -117,13 +80,55 @@ function findNextNodeDownward( return undefined } - const parentNodes = sortedNodes(settings, parent) - const nodeIdx = parentNodes.findIndex(n => n.path() === treeNode.path()) - - const nextNode = parentNodes[nodeIdx + 1] - if (nextNode) { - return nextNode + const neighborNodes = sortedNodes(settings, parent) + const nodeIdx = neighborNodes.findIndex(n => n.path() === treeNode.path()) + if (nodeIdx === 0) { + return parent + } + const upwardNeighbor = neighborNodes[nodeIdx - 1] + if (upwardNeighbor) { + return lastVisibleChild(settings, upwardNeighbor) } else { - return findNextNodeDownward(settings, parent) + return findNextNodeUpward(settings, parent) + } +} + +function lastVisibleChild(settings: SettingsState, treeNode: q.TreeNode): q.TreeNode { + const nodes = sortedNodes(settings, treeNode).filter(isTreeNodeVisible) + if (nodes.length === 0) { + return treeNode + } + return lastVisibleChild(settings, nodes[nodes.length - 1]) +} + +function findNextNodeDownward( + settings: SettingsState, + treeNode: q.TreeNode +): q.TreeNode | undefined { + const children = sortedNodes(settings, treeNode).filter(isTreeNodeVisible) + const firstChild = children[0] + if (firstChild) { + return firstChild + } + + return findNextNodeDownwardNeighbor(settings, treeNode) +} + +function findNextNodeDownwardNeighbor( + settings: SettingsState, + treeNode: q.TreeNode +): q.TreeNode | undefined { + const parent = treeNode.sourceEdge && treeNode.sourceEdge.source + if (!parent) { + return undefined + } + + const neighborNodes = sortedNodes(settings, parent).filter(isTreeNodeVisible) + const nodeIdx = neighborNodes.findIndex(n => n.path() === treeNode.path()) + const downwardNeighbor = neighborNodes[nodeIdx + 1] + if (downwardNeighbor) { + return downwardNeighbor + } else { + return findNextNodeDownwardNeighbor(settings, parent) } } diff --git a/app/src/components/Tree/TreeNode.tsx b/app/src/components/Tree/TreeNode.tsx index f2262b9..6d8e56c 100644 --- a/app/src/components/Tree/TreeNode.tsx +++ b/app/src/components/Tree/TreeNode.tsx @@ -67,8 +67,8 @@ function useIsAllowedToAutoExpandState(props: Props): boolean { const [isAllowedToAutoExpand, setAllowAutoExpand] = useState(false) useEffect(() => { - const newIsAllowedToAutoExpand = !(treeNode.edgeCount() > settings.get('autoExpandLimit')) - if (!isRoot && newIsAllowedToAutoExpand !== isAllowedToAutoExpand) { + const newIsAllowedToAutoExpand = isRoot || treeNode.edgeCount() <= settings.get('autoExpandLimit') + if (newIsAllowedToAutoExpand !== isAllowedToAutoExpand) { setAllowAutoExpand(newIsAllowedToAutoExpand) } }, [treeNode.edgeCount(), settings.get('autoExpandLimit')]) @@ -77,45 +77,56 @@ function useIsAllowedToAutoExpandState(props: Props): boolean { } function TreeNodeComponent(props: Props) { - const { classes, className, settings, theme, treeNode, lastUpdate } = props + const { classes, className, settings, theme, treeNode, lastUpdate, name } = props - const [animationDirty, setAnimationDirty] = useState(false) - const [collapsed, setCollapsed] = useState(props.collapsed) - const [cssAnimationWasSetAt, setCssAnimationWasSetAt] = useState(0) - const [willUpdateTime, setWillUpdateTime] = useState(performance.now()) + const [showUpdateAnimation, setShowUpdateAnimation] = useState(false) + const [collapsedOverride, setCollapsedOverride] = useState(undefined) const [isHovering, setIsHovering] = useState(false) const [selected, setSelected] = useState(false) const nodeRef = useRef() const isAllowedToAutoExpand = useIsAllowedToAutoExpandState(props) - useViewModelSubscriptions(treeNode, nodeRef, setSelected, setCollapsed) + useViewModelSubscriptions(treeNode, nodeRef, setSelected, setCollapsedOverride) + useAnimationToIndicateTopicUpdate(lastUpdate, setShowUpdateAnimation, showUpdateAnimation) + + const isCollapsed = + Boolean(collapsedOverride) === collapsedOverride ? Boolean(collapsedOverride) : !isAllowedToAutoExpand const setHover = debounce((hover: boolean) => { setIsHovering(hover) }, 45) const toggle = useCallback(() => { - setCollapsed(!collapsed) - }, [collapsed]) + setCollapsedOverride(!isCollapsed) + }, [isCollapsed]) const didSelectTopic = useCallback( (event?: React.MouseEvent) => { - console.log('Did select', treeNode.path()) - console.log(event) event && event.stopPropagation() props.selectTopicAction(treeNode) }, [treeNode] ) - const didClickTitle = (event: React.MouseEvent) => { - event.stopPropagation() - didSelectTopic() - toggle() - } + const didClickTitle = React.useCallback( + (event: React.MouseEvent) => { + event.stopPropagation() + didSelectTopic() + toggle() + }, + [toggle, didSelectTopic] + ) + + const toggleCollapsed = useCallback( + (event: React.MouseEvent) => { + event.stopPropagation() + toggle() + }, + [toggle] + ) const didObtainFocus = useCallback(() => { didSelectTopic() - }, []) + }, [didSelectTopic]) const mouseOver = (event: React.MouseEvent) => { event.stopPropagation() @@ -130,23 +141,13 @@ function TreeNodeComponent(props: Props) { setHover(false) } - const toggleCollapsed = useCallback( - (event: React.MouseEvent) => { - event.stopPropagation() - toggle() - }, - [toggle] - ) - useEffect(() => { - treeNode.viewModel && treeNode.viewModel.setExpanded(!collapsed, false) - }, [collapsed]) + treeNode.viewModel && treeNode.viewModel.setExpanded(!isCollapsed, false) + }, [isCollapsed]) return useMemo(() => { - const shouldBeRenderedCollapsed = Boolean(collapsed) === collapsed ? Boolean(collapsed) : !isAllowedToAutoExpand - function renderNodes() { - if (shouldBeRenderedCollapsed) { + if (isCollapsed) { return null } @@ -160,14 +161,13 @@ function TreeNodeComponent(props: Props) { ) } - const shouldStartAnimation = settings.get('highlightTopicUpdates') + const shouldStartAnimation = settings.get('highlightTopicUpdates') && showUpdateAnimation const animationName = theme.palette.type === 'light' ? 'updateLight' : 'updateDark' const animation = shouldStartAnimation ? { willChange: 'auto', translateZ: 0, animation: `${animationName} 0.5s` } : {} const highlightClass = selected ? classes.selected : isHovering ? classes.hover : '' - return (
@@ -192,7 +192,26 @@ function TreeNodeComponent(props: Props) { {renderNodes()}
) - }, [lastUpdate, treeNode, collapsed, selected, isAllowedToAutoExpand, isHovering]) + }, [lastUpdate, treeNode, name, isCollapsed, selected, showUpdateAnimation, isHovering]) } export default withStyles(styles, { withTheme: true })(TreeNodeComponent) +function useAnimationToIndicateTopicUpdate( + lastUpdate: number, + setShowUpdateAnimation: React.Dispatch>, + showUpdateAnimation: boolean +) { + useEffect(() => { + if (Date.now() - lastUpdate < 3000) { + setShowUpdateAnimation(true) + } + }, [lastUpdate]) + useEffect(() => { + if (showUpdateAnimation) { + const timeout = setTimeout(() => setShowUpdateAnimation(false), 500) + return function cleanup() { + clearTimeout(timeout) + } + } + }, [showUpdateAnimation]) +} diff --git a/app/src/components/Tree/useViewModelSubscriptions.tsx b/app/src/components/Tree/useViewModelSubscriptions.tsx index 2234a0b..c231d4e 100644 --- a/app/src/components/Tree/useViewModelSubscriptions.tsx +++ b/app/src/components/Tree/useViewModelSubscriptions.tsx @@ -25,7 +25,7 @@ export function useViewModelSubscriptions( return function cleanup() { removeSubscriber() } - }) + }, [treeNode]) function addSubscriber() { treeNode.viewModel = new TopicViewModel()