From 93776944b901b2fad7bef8961771dc674938f6a2 Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Thu, 2 Apr 2026 18:35:27 +0200 Subject: [PATCH] refactor: extract keyboard handlers to reduce cognitive complexity (S3776) Extract per-key arrow handler logic into standalone functions outside the component in SidebarTree.tsx and TreeView.tsx, reducing handleKeyDown cognitive complexity from 31 to below the 15-unit maximum. Co-Authored-By: Claude Sonnet 4.6 --- .../composites/TreeView/TreeView.tsx | 115 ++++++++---------- .../layout/Sidebar/SidebarTree.tsx | 111 ++++++++--------- 2 files changed, 106 insertions(+), 120 deletions(-) diff --git a/src/design-system/composites/TreeView/TreeView.tsx b/src/design-system/composites/TreeView/TreeView.tsx index f16d2fa..056dc46 100644 --- a/src/design-system/composites/TreeView/TreeView.tsx +++ b/src/design-system/composites/TreeView/TreeView.tsx @@ -31,6 +31,52 @@ function flattenVisibleNodes( return result } +// ── Keyboard nav helpers ───────────────────────────────────────────────────── + +function handleArrowDown(visibleNodes: FlatNode[], currentIndex: number, focusNode: (id: string) => void) { + const next = visibleNodes[currentIndex + 1] + if (next) focusNode(next.node.id) +} + +function handleArrowUp(visibleNodes: FlatNode[], currentIndex: number, focusNode: (id: string) => void) { + const prev = visibleNodes[currentIndex - 1] + if (prev) focusNode(prev.node.id) +} + +function handleArrowRight( + current: FlatNode | undefined, + currentIndex: number, + expandedSet: Set, + visibleNodes: FlatNode[], + handleToggle: (id: string) => void, + focusNode: (id: string) => void, +) { + if (!current) return + const hasChildren = current.node.children && current.node.children.length > 0 + if (!hasChildren) return + if (!expandedSet.has(current.node.id)) { + handleToggle(current.node.id) + } else { + const next = visibleNodes[currentIndex + 1] + if (next) focusNode(next.node.id) + } +} + +function handleArrowLeft( + current: FlatNode | undefined, + expandedSet: Set, + handleToggle: (id: string) => void, + focusNode: (id: string) => void, +) { + if (!current) return + const hasChildren = current.node.children && current.node.children.length > 0 + if (hasChildren && expandedSet.has(current.node.id)) { + handleToggle(current.node.id) + } else if (current.parentId !== null) { + focusNode(current.parentId) + } +} + interface TreeViewProps { nodes: TreeNode[] onSelect?: (id: string) => void @@ -105,68 +151,13 @@ export function TreeView({ const current = visibleNodes[currentIndex] switch (e.key) { - case 'ArrowDown': { - e.preventDefault() - const next = visibleNodes[currentIndex + 1] - if (next) focusNode(next.node.id) - break - } - case 'ArrowUp': { - e.preventDefault() - const prev = visibleNodes[currentIndex - 1] - if (prev) focusNode(prev.node.id) - break - } - case 'ArrowRight': { - e.preventDefault() - if (!current) break - const hasChildren = current.node.children && current.node.children.length > 0 - if (hasChildren) { - if (!expandedSet.has(current.node.id)) { - // Expand it - handleToggle(current.node.id) - } else { - // Move to first child (it will be the next visible node) - const next = visibleNodes[currentIndex + 1] - if (next) focusNode(next.node.id) - } - } - break - } - case 'ArrowLeft': { - e.preventDefault() - if (!current) break - const hasChildren = current.node.children && current.node.children.length > 0 - if (hasChildren && expandedSet.has(current.node.id)) { - // Collapse - handleToggle(current.node.id) - } else if (current.parentId !== null) { - // Move to parent - focusNode(current.parentId) - } - break - } - case 'Enter': { - e.preventDefault() - if (current) { - onSelect?.(current.node.id) - } - break - } - case 'Home': { - e.preventDefault() - if (visibleNodes.length > 0) { - focusNode(visibleNodes[0].node.id) - } - break - } - case 'End': { - e.preventDefault() - if (visibleNodes.length > 0) { - focusNode(visibleNodes[visibleNodes.length - 1].node.id) - } - break - } + case 'ArrowDown': { e.preventDefault(); handleArrowDown(visibleNodes, currentIndex, focusNode); break } + case 'ArrowUp': { e.preventDefault(); handleArrowUp(visibleNodes, currentIndex, focusNode); break } + case 'ArrowRight': { e.preventDefault(); handleArrowRight(current, currentIndex, expandedSet, visibleNodes, handleToggle, focusNode); break } + case 'ArrowLeft': { e.preventDefault(); handleArrowLeft(current, expandedSet, handleToggle, focusNode); break } + case 'Enter': { e.preventDefault(); if (current) onSelect?.(current.node.id); break } + case 'Home': { e.preventDefault(); if (visibleNodes.length > 0) focusNode(visibleNodes[0].node.id); break } + case 'End': { e.preventDefault(); if (visibleNodes.length > 0) focusNode(visibleNodes[visibleNodes.length - 1].node.id); break } } }, // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/src/design-system/layout/Sidebar/SidebarTree.tsx b/src/design-system/layout/Sidebar/SidebarTree.tsx index fce531a..4fa9770 100644 --- a/src/design-system/layout/Sidebar/SidebarTree.tsx +++ b/src/design-system/layout/Sidebar/SidebarTree.tsx @@ -124,6 +124,52 @@ function filterNodes( return { filtered: walk(nodes), matchedParentIds } } +// ── Keyboard nav helpers ───────────────────────────────────────────────────── + +function handleArrowDown(visibleNodes: FlatNode[], currentIndex: number, focusNode: (id: string) => void) { + const next = visibleNodes[currentIndex + 1] + if (next) focusNode(next.node.id) +} + +function handleArrowUp(visibleNodes: FlatNode[], currentIndex: number, focusNode: (id: string) => void) { + const prev = visibleNodes[currentIndex - 1] + if (prev) focusNode(prev.node.id) +} + +function handleArrowRight( + current: FlatNode | undefined, + currentIndex: number, + expandedSet: Set, + visibleNodes: FlatNode[], + handleToggle: (id: string) => void, + focusNode: (id: string) => void, +) { + if (!current) return + const hasChildren = current.node.children && current.node.children.length > 0 + if (!hasChildren) return + if (!expandedSet.has(current.node.id)) { + handleToggle(current.node.id) + } else { + const next = visibleNodes[currentIndex + 1] + if (next) focusNode(next.node.id) + } +} + +function handleArrowLeft( + current: FlatNode | undefined, + expandedSet: Set, + handleToggle: (id: string) => void, + focusNode: (id: string) => void, +) { + if (!current) return + const hasChildren = current.node.children && current.node.children.length > 0 + if (hasChildren && expandedSet.has(current.node.id)) { + handleToggle(current.node.id) + } else if (current.parentId !== null) { + focusNode(current.parentId) + } +} + // ── SidebarTree ────────────────────────────────────────────────────────────── export function SidebarTree({ @@ -222,64 +268,13 @@ export function SidebarTree({ const current = visibleNodes[currentIndex] switch (e.key) { - case 'ArrowDown': { - e.preventDefault() - const next = visibleNodes[currentIndex + 1] - if (next) focusNode(next.node.id) - break - } - case 'ArrowUp': { - e.preventDefault() - const prev = visibleNodes[currentIndex - 1] - if (prev) focusNode(prev.node.id) - break - } - case 'ArrowRight': { - e.preventDefault() - if (!current) break - const hasChildren = current.node.children && current.node.children.length > 0 - if (hasChildren) { - if (!expandedSet.has(current.node.id)) { - handleToggle(current.node.id) - } else { - const next = visibleNodes[currentIndex + 1] - if (next) focusNode(next.node.id) - } - } - break - } - case 'ArrowLeft': { - e.preventDefault() - if (!current) break - const hasChildren = current.node.children && current.node.children.length > 0 - if (hasChildren && expandedSet.has(current.node.id)) { - handleToggle(current.node.id) - } else if (current.parentId !== null) { - focusNode(current.parentId) - } - break - } - case 'Enter': { - e.preventDefault() - if (current?.node.path) { - navigate(current.node.path) - } - break - } - case 'Home': { - e.preventDefault() - if (visibleNodes.length > 0) { - focusNode(visibleNodes[0].node.id) - } - break - } - case 'End': { - e.preventDefault() - if (visibleNodes.length > 0) { - focusNode(visibleNodes[visibleNodes.length - 1].node.id) - } - break - } + case 'ArrowDown': { e.preventDefault(); handleArrowDown(visibleNodes, currentIndex, focusNode); break } + case 'ArrowUp': { e.preventDefault(); handleArrowUp(visibleNodes, currentIndex, focusNode); break } + case 'ArrowRight': { e.preventDefault(); handleArrowRight(current, currentIndex, expandedSet, visibleNodes, handleToggle, focusNode); break } + case 'ArrowLeft': { e.preventDefault(); handleArrowLeft(current, expandedSet, handleToggle, focusNode); break } + case 'Enter': { e.preventDefault(); if (current?.node.path) navigate(current.node.path); break } + case 'Home': { e.preventDefault(); if (visibleNodes.length > 0) focusNode(visibleNodes[0].node.id); break } + case 'End': { e.preventDefault(); if (visibleNodes.length > 0) focusNode(visibleNodes[visibleNodes.length - 1].node.id); break } } }, // eslint-disable-next-line react-hooks/exhaustive-deps