From 4b86c696a5c0f86e95db5b01f2b6ecadb6b6cdeb Mon Sep 17 00:00:00 2001 From: Lum1104 Date: Mon, 4 May 2026 19:06:39 +0800 Subject: [PATCH] fix(dashboard): tour navigation glitches across layers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four related issues that surfaced while walking the Learn-mode tour through a multi-layer project (microservices-demo): 1. Tour auto-expand never released. The tour effect that expands highlighted nodes' containers had no corresponding collapse when the step changed, so containers accumulated open as the user advanced. Track the set of containers we expanded and release any not needed by the current step; user-toggled containers are never tracked here, so they're never auto-collapsed. 2. Manual container toggle yanked off-screen. Stage 2 reflow shifted the just-clicked container away from the cursor. `toggleContainer` now records `pendingFocusContainer` on expand; GraphView locks the viewport onto that container's centre with the current zoom so it appears to expand in place. 3. Tour fitView fired before highlighted children existed. A single RAF after `tourHighlightedNodeIds` change wasn't enough — child nodes only appear once Stage 2 layout writes `containerLayoutCache`, and React Flow only knows their absolute position after a measure pass. `useNodes()` doesn't fire on measure completion, so we poll `getInternalNode().measured` each frame (up to ~4s) and call `fitView({ nodes })` once every highlight is measured, with `maxZoom: 1.2 / minZoom: 0.4`. While waiting, a new `tourFitPending` flag drives a "Locating tour highlight…" overlay so the user knows the layout is still settling. 4. Cross-layer tour transitions reused stale Stage 2 cache. Container ids derive from per-layer state (folder names in folder strategy, `container:cluster-N` in community strategy) and collide across layers — API Contracts and Load Testing both produce `container:cluster-0`. `setTourStep` / `nextTourStep` / `prevTourStep` / `startTour` didn't reset the container caches the way `drillIntoLayer` does, so when tour crossed a layer the new layer's expanded containers hit the previous layer's cache, Stage 2 skipped its rerun, and children never showed. Extracted `layerResetIfChanged` and applied it in all four tour actions. Verified end-to-end against microservices-demo via headless Chrome: all 15 tour steps now expand the right container(s), zoom onto the referenced files, and collapse the previous step's auto-expansions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../dashboard/src/components/GraphView.tsx | 155 +++++++++++++++--- .../packages/dashboard/src/store.ts | 66 +++++++- 2 files changed, 191 insertions(+), 30 deletions(-) diff --git a/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx b/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx index 0caaf98..91e5e18 100644 --- a/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx +++ b/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx @@ -78,32 +78,83 @@ const NODE_TYPE_TO_CATEGORY: Record = { // ── Helper components that must live inside ──────────────── -/** Pans/zooms to tour-highlighted nodes. */ +/** + * Pans/zooms to tour-highlighted nodes. Highlighted nodes are usually + * children of collapsed containers — auto-expand fires synchronously on + * the same `tourHighlightedNodeIds` change, but their child entries don't + * appear in React Flow's node list until Stage 2 layout writes the + * `containerLayoutCache` (async ELK call, hundreds of ms on big layers). + * + * We subscribe to React Flow's reactive node list via `useNodes()` so the + * effect re-runs every time the node set actually changes (Stage 1, Stage + * 2, expand/collapse). When every highlighted id is present we fit; until + * then we wait. A 2s fallback timer covers the case where a highlighted + * id is filtered out and never materialises. + */ function TourFitView() { const tourHighlightedNodeIds = useDashboardStore((s) => s.tourHighlightedNodeIds); - const { fitView } = useReactFlow(); - const prevRef = useRef([]); + const setTourFitPending = useDashboardStore((s) => s.setTourFitPending); + const { fitView, getInternalNode } = useReactFlow(); + const fittedKeyRef = useRef(""); useEffect(() => { - const prev = prevRef.current; - const changed = - tourHighlightedNodeIds.length > 0 && - (tourHighlightedNodeIds.length !== prev.length || - tourHighlightedNodeIds.some((id, i) => id !== prev[i])); - prevRef.current = tourHighlightedNodeIds; + const targetKey = tourHighlightedNodeIds.join("\n"); + if (targetKey === "") { + fittedKeyRef.current = ""; + setTourFitPending(false); + return; + } + if (targetKey === fittedKeyRef.current) return; - if (changed) { - requestAnimationFrame(() => { + // Wait for React Flow to finish Stage 2 layout AND post-mount measure + // before fitting. We poll the internal lookup directly because + // `useNodes()` reflects user-supplied nodes only and doesn't fire on + // measure completion. Once every highlighted id has measured + // dimensions, hand the ids to fitView — React Flow handles the + // child→absolute coordinate transform itself, which is more reliable + // than recomputing bbox manually. + const MAX_FRAMES = 240; // ~4s at 60fps + let frame = 0; + let cancelled = false; + let rafId = 0; + setTourFitPending(true); + + const tick = () => { + if (cancelled) return; + let ready = true; + for (const id of tourHighlightedNodeIds) { + const internal = getInternalNode(id); + if (!internal || !internal.measured?.width || !internal.measured?.height) { + ready = false; + break; + } + } + if (ready) { fitView({ nodes: tourHighlightedNodeIds.map((id) => ({ id })), duration: 500, padding: 0.3, maxZoom: 1.2, - minZoom: 0.01, + minZoom: 0.4, }); - }); - } - }, [tourHighlightedNodeIds, fitView]); + fittedKeyRef.current = targetKey; + setTourFitPending(false); + return; + } + if (++frame < MAX_FRAMES) { + rafId = requestAnimationFrame(tick); + return; + } + fitView({ duration: 500, padding: 0.3 }); + fittedKeyRef.current = targetKey; + setTourFitPending(false); + }; + rafId = requestAnimationFrame(tick); + return () => { + cancelled = true; + cancelAnimationFrame(rafId); + }; + }, [tourHighlightedNodeIds, fitView, getInternalNode, setTourFitPending]); return null; } @@ -1219,6 +1270,10 @@ function GraphViewInner() { const setReactFlowInstance = useDashboardStore((s) => s.setReactFlowInstance); const tourHighlightedNodeIds = useDashboardStore((s) => s.tourHighlightedNodeIds); const expandContainer = useDashboardStore((s) => s.expandContainer); + const collapseContainer = useDashboardStore((s) => s.collapseContainer); + const pendingFocusContainer = useDashboardStore((s) => s.pendingFocusContainer); + const setPendingFocusContainer = useDashboardStore((s) => s.setPendingFocusContainer); + const tourFitPending = useDashboardStore((s) => s.tourFitPending); const { preset } = useTheme(); const overviewGraph = useOverviewGraph(); @@ -1237,7 +1292,7 @@ function GraphViewInner() { const [nodes, setNodes, onNodesChange] = useNodesState(initialNodes); const [edges, setEdges, onEdgesChange] = useEdgesState(initialEdges); - const { fitView, getViewport } = useReactFlow(); + const { fitView, getViewport, setCenter } = useReactFlow(); useEffect(() => { setNodes(initialNodes); @@ -1267,6 +1322,33 @@ function GraphViewInner() { return () => cancelAnimationFrame(raf); }, [nodes, fitView]); + // Lock viewport onto a container the user just manually expanded so it + // appears to expand in place rather than getting yanked off-screen by + // the surrounding ELK reflow. Re-runs as nodes update (Stage 2 may + // shift positions a few times) and clears itself after a short window + // so subsequent layout shifts stop hijacking the viewport. + useEffect(() => { + if (!pendingFocusContainer) return; + const node = nodes.find((n) => n.id === pendingFocusContainer); + if (!node) return; + const w = + (node.width as number | undefined) ?? + ((node.style?.width as number | undefined) ?? 0); + const h = + (node.height as number | undefined) ?? + ((node.style?.height as number | undefined) ?? 0); + const cx = node.position.x + w / 2; + const cy = node.position.y + h / 2; + const { zoom } = getViewport(); + setCenter(cx, cy, { zoom, duration: 0 }); + }, [pendingFocusContainer, nodes, getViewport, setCenter]); + + useEffect(() => { + if (!pendingFocusContainer) return; + const t = window.setTimeout(() => setPendingFocusContainer(null), 1200); + return () => window.clearTimeout(t); + }, [pendingFocusContainer, setPendingFocusContainer]); + // ── Auto-expand triggers (Task 13) ───────────────────────────────────── // Only meaningful in layer-detail; in overview mode there are no // containers so all three effects no-op. @@ -1281,15 +1363,40 @@ function GraphViewInner() { if (cid && cid !== focusNodeId) expandContainer(cid); }, [focusNodeId, nodeToContainer, expandContainer]); - // Tour: expand containers for every tour-highlighted node so the tour - // can fitView onto real nodes rather than collapsed atoms. + // Tour: expand containers needed for the current step, and release any + // containers we expanded for the previous step that aren't needed now. + // Containers the user expanded manually aren't tracked here, so they're + // never auto-collapsed. stopTour resets tourHighlightedNodeIds to [], + // which falls through to the "release all" branch. + const tourBorrowedContainersRef = useRef>(new Set()); useEffect(() => { - if (tourHighlightedNodeIds.length === 0 || !nodeToContainer) return; + if (!nodeToContainer) return; + + const needed = new Set(); for (const nid of tourHighlightedNodeIds) { const cid = nodeToContainer.get(nid); - if (cid && cid !== nid) expandContainer(cid); + if (cid && cid !== nid) needed.add(cid); } - }, [tourHighlightedNodeIds, nodeToContainer, expandContainer]); + + const stillBorrowed = new Set(); + for (const cid of tourBorrowedContainersRef.current) { + if (needed.has(cid)) { + stillBorrowed.add(cid); + } else { + collapseContainer(cid); + } + } + + const expandedNow = useDashboardStore.getState().expandedContainers; + for (const cid of needed) { + if (!expandedNow.has(cid)) { + expandContainer(cid); + stillBorrowed.add(cid); + } + } + + tourBorrowedContainersRef.current = stillBorrowed; + }, [tourHighlightedNodeIds, nodeToContainer, expandContainer, collapseContainer]); // Zoom: debounced auto-expand when the user has zoomed in past 1.0. // Hysteresis: zoom < 0.6 = no auto-expand AND no auto-collapse (v1, the @@ -1406,7 +1513,7 @@ function GraphViewInner() { - {layoutStatus === "computing" && ( + {(layoutStatus === "computing" || tourFitPending) && (
- Computing layout… + + {tourFitPending ? "Locating tour highlight…" : "Computing layout…"} +
)} diff --git a/understand-anything-plugin/packages/dashboard/src/store.ts b/understand-anything-plugin/packages/dashboard/src/store.ts index b635d66..bd5a0c9 100644 --- a/understand-anything-plugin/packages/dashboard/src/store.ts +++ b/understand-anything-plugin/packages/dashboard/src/store.ts @@ -196,7 +196,14 @@ interface DashboardStore { expandedContainers: Set; toggleContainer: (containerId: string) => void; expandContainer: (containerId: string) => void; + collapseContainer: (containerId: string) => void; collapseAllContainers: () => void; + /** Container the user just manually expanded; viewport should lock onto it. Cleared by GraphView once the lock is applied. */ + pendingFocusContainer: string | null; + setPendingFocusContainer: (containerId: string | null) => void; + /** True while TourFitView is waiting for highlighted nodes to materialise (Stage 2 layout in progress). Drives the "Computing layout…" overlay. */ + tourFitPending: boolean; + setTourFitPending: (pending: boolean) => void; containerLayoutCache: Map< string, @@ -245,6 +252,28 @@ function navigateTourToLayer( return {}; } +/** + * Container ids derive from per-layer state — folder names in folder-strategy + * layers, community indices (`container:cluster-N`) in community-strategy + * layers — and collide across layers (e.g. API Contracts and Load Testing + * both produce `container:cluster-0`). When a tour step crosses layers we + * must drop the previous layer's container caches so Stage 2 actually re- + * runs for the new layer's children. Mirrors the reset block in + * `drillIntoLayer`. + */ +function layerResetIfChanged( + layerNav: Partial, + prevLayerId: string | null, +): Partial { + const next = layerNav.activeLayerId; + if (!next || next === prevLayerId) return {}; + return { + containerLayoutCache: new Map(), + containerSizeMemory: new Map(), + expandedContainers: new Set(), + }; +} + export const useDashboardStore = create()((set, get) => ({ graph: null, nodesById: new Map(), @@ -532,7 +561,7 @@ export const useDashboardStore = create()((set, get) => ({ }, startTour: () => { - const { graph, nodeIdToLayerId } = get(); + const { graph, nodeIdToLayerId, activeLayerId } = get(); if (!graph || !graph.tour || graph.tour.length === 0) return; const sorted = getSortedTour(graph); const layerNav = navigateTourToLayer(nodeIdToLayerId, sorted[0].nodeIds); @@ -542,6 +571,7 @@ export const useDashboardStore = create()((set, get) => ({ tourHighlightedNodeIds: sorted[0].nodeIds, selectedNodeId: null, ...layerNav, + ...layerResetIfChanged(layerNav, activeLayerId), }); }, @@ -553,7 +583,7 @@ export const useDashboardStore = create()((set, get) => ({ }), setTourStep: (step) => { - const { graph, nodeIdToLayerId } = get(); + const { graph, nodeIdToLayerId, activeLayerId } = get(); if (!graph || !graph.tour || graph.tour.length === 0) return; const sorted = getSortedTour(graph); if (step < 0 || step >= sorted.length) return; @@ -562,11 +592,12 @@ export const useDashboardStore = create()((set, get) => ({ currentTourStep: step, tourHighlightedNodeIds: sorted[step].nodeIds, ...layerNav, + ...layerResetIfChanged(layerNav, activeLayerId), }); }, nextTourStep: () => { - const { graph, currentTourStep, nodeIdToLayerId } = get(); + const { graph, currentTourStep, nodeIdToLayerId, activeLayerId } = get(); if (!graph || !graph.tour || graph.tour.length === 0) return; const sorted = getSortedTour(graph); if (currentTourStep < sorted.length - 1) { @@ -576,12 +607,13 @@ export const useDashboardStore = create()((set, get) => ({ currentTourStep: next, tourHighlightedNodeIds: sorted[next].nodeIds, ...layerNav, + ...layerResetIfChanged(layerNav, activeLayerId), }); } }, prevTourStep: () => { - const { graph, currentTourStep, nodeIdToLayerId } = get(); + const { graph, currentTourStep, nodeIdToLayerId, activeLayerId } = get(); if (!graph || !graph.tour || graph.tour.length === 0) return; if (currentTourStep > 0) { const sorted = getSortedTour(graph); @@ -591,6 +623,7 @@ export const useDashboardStore = create()((set, get) => ({ currentTourStep: prev, tourHighlightedNodeIds: sorted[prev].nodeIds, ...layerNav, + ...layerResetIfChanged(layerNav, activeLayerId), }); } }, @@ -641,12 +674,23 @@ export const useDashboardStore = create()((set, get) => ({ }, expandedContainers: new Set(), + pendingFocusContainer: null, + setPendingFocusContainer: (containerId) => + set({ pendingFocusContainer: containerId }), + tourFitPending: false, + setTourFitPending: (pending) => set({ tourFitPending: pending }), toggleContainer: (containerId) => set((state) => { const next = new Set(state.expandedContainers); - if (next.has(containerId)) next.delete(containerId); - else next.add(containerId); - return { expandedContainers: next }; + const willExpand = !next.has(containerId); + if (willExpand) next.add(containerId); + else next.delete(containerId); + return { + expandedContainers: next, + pendingFocusContainer: willExpand + ? containerId + : state.pendingFocusContainer, + }; }), expandContainer: (containerId) => set((state) => { @@ -655,6 +699,13 @@ export const useDashboardStore = create()((set, get) => ({ next.add(containerId); return { expandedContainers: next }; }), + collapseContainer: (containerId) => + set((state) => { + if (!state.expandedContainers.has(containerId)) return {}; + const next = new Set(state.expandedContainers); + next.delete(containerId); + return { expandedContainers: next }; + }), collapseAllContainers: () => set({ expandedContainers: new Set() }), containerLayoutCache: new Map(), @@ -689,3 +740,4 @@ export const useDashboardStore = create()((set, get) => ({ }), clearLayoutIssues: () => set({ layoutIssues: [] }), })); +