feat(FN-4036): fix zoom-aware graph pan bounds
Fixed zoom-aware graph pan bounds in the dependency graph plugin, correcting edge-case behavior when the graph is zoomed or panned. Added regression tests for graph interactions and updated documentation to cover the corrected behavior. Fusion-Task-Id: FN-4036
This commit is contained in:
@@ -20,7 +20,8 @@ The dependency graph view is registered as a **bundled plugin view** in the dash
|
||||
- **Auto-layout**: Sugiyama-style layered layout (`computeAutoLayout`) groups nodes by dependency depth and spaces layers consistently
|
||||
- **Edge drawing**: SVG bezier curves from source bottom-center to target top-center, with arrowheads showing dependent → dependency direction
|
||||
- **Interaction**: drag-to-pan canvas background, drag-to-reposition nodes, cursor-centered wheel zoom, pinch-to-zoom with stationary midpoint, keyboard shortcuts, zoom toolbar, reset, and fit-to-graph
|
||||
- **Fit-to-graph**: computes node bounding box with layout node dimensions and applies zoom/pan so the graph fits in viewport with padding
|
||||
- **Zoomed navigation reachability**: pan clamping now scales with the full rendered graph bounds (min/max node extents) at the active zoom level, so zooming in no longer traps off-screen content behind fixed viewport-only limits
|
||||
- **Fit-to-graph**: computes node bounding box from both minimum and maximum node coordinates (including negative auto-layout origins) with layout node dimensions and applies zoom/pan so the graph fits in viewport with padding
|
||||
- **Initial auto-fit**: when no saved scoped positions exist, the first non-empty render auto-fits once; subsequent updates preserve user navigation state
|
||||
- **Position persistence**: dragged node positions are stored per project in browser localStorage and restored on reload
|
||||
- **Animated transitions**: fit/reset operations animate `transform` (`var(--transition-normal)`), while continuous drag/wheel/pinch stays transition-free for responsiveness
|
||||
|
||||
@@ -100,6 +100,7 @@ export function DependencyGraph({
|
||||
onPointerUp,
|
||||
onWheelZoom,
|
||||
handleKeyDown,
|
||||
setGraphBounds,
|
||||
} = useGraphInteraction();
|
||||
|
||||
useEffect(() => {
|
||||
@@ -121,12 +122,38 @@ export function DependencyGraph({
|
||||
|
||||
const bounds = useMemo(() => {
|
||||
const values = Array.from(positions.values());
|
||||
if (values.length === 0) return { width: 0, height: 0 };
|
||||
if (values.length === 0) {
|
||||
return { minX: 0, minY: 0, maxX: 0, maxY: 0, width: 0, height: 0 };
|
||||
}
|
||||
|
||||
const minX = Math.min(...values.map((pos) => pos.x));
|
||||
const minY = Math.min(...values.map((pos) => pos.y));
|
||||
const maxX = Math.max(...values.map((pos) => pos.x + NODE_WIDTH));
|
||||
const maxY = Math.max(...values.map((pos) => pos.y + NODE_HEIGHT));
|
||||
return { width: maxX, height: maxY };
|
||||
|
||||
return {
|
||||
minX,
|
||||
minY,
|
||||
maxX,
|
||||
maxY,
|
||||
width: Math.max(0, maxX - minX),
|
||||
height: Math.max(0, maxY - minY),
|
||||
};
|
||||
}, [positions]);
|
||||
|
||||
const normalizedPositions = useMemo(() => {
|
||||
if (positions.size === 0) return positions;
|
||||
const next = new Map<string, { x: number; y: number }>();
|
||||
for (const [taskId, position] of positions.entries()) {
|
||||
next.set(taskId, { x: position.x - bounds.minX, y: position.y - bounds.minY });
|
||||
}
|
||||
return next;
|
||||
}, [bounds.minX, bounds.minY, positions]);
|
||||
|
||||
useEffect(() => {
|
||||
setGraphBounds({ minX: 0, minY: 0, maxX: bounds.width, maxY: bounds.height });
|
||||
}, [bounds.height, bounds.width, setGraphBounds]);
|
||||
|
||||
const handleResetLayout = useCallback(() => {
|
||||
clearSavedPositions();
|
||||
const freshLayout = computeAutoLayout(graphData, { nodeWidth: NODE_WIDTH, nodeHeight: NODE_HEIGHT, horizontalGap: 40, verticalGap: 80 });
|
||||
@@ -189,7 +216,7 @@ export function DependencyGraph({
|
||||
onKeyDown={(event) => {
|
||||
const viewport = viewportRef.current;
|
||||
if (!viewport) return;
|
||||
handleKeyDown(event, viewport.clientWidth, viewport.clientHeight, positions, { nodeWidth: NODE_WIDTH, nodeHeight: NODE_HEIGHT });
|
||||
handleKeyDown(event, viewport.clientWidth, viewport.clientHeight, normalizedPositions, { nodeWidth: NODE_WIDTH, nodeHeight: NODE_HEIGHT });
|
||||
}}
|
||||
tabIndex={0}
|
||||
onClick={() => {
|
||||
@@ -203,7 +230,7 @@ export function DependencyGraph({
|
||||
<div className={`graph-canvas-transform${transitioning ? " graph-canvas-transform--animate" : ""}`} style={{ transform, width: `${bounds.width}px`, height: `${bounds.height}px` }}>
|
||||
<GraphEdges
|
||||
edges={graphData.edges}
|
||||
positions={positions}
|
||||
positions={normalizedPositions}
|
||||
nodeWidth={NODE_WIDTH}
|
||||
nodeHeight={NODE_HEIGHT}
|
||||
highlightedEdgeIds={
|
||||
@@ -218,7 +245,7 @@ export function DependencyGraph({
|
||||
/>
|
||||
<div className="dependency-graph__nodes-layer">
|
||||
{graphData.nodes.map((node) => {
|
||||
const position = positions.get(node.task.id);
|
||||
const position = normalizedPositions.get(node.task.id);
|
||||
if (!position) return null;
|
||||
|
||||
return (
|
||||
@@ -232,10 +259,11 @@ export function DependencyGraph({
|
||||
scale={zoom}
|
||||
onNodePositionChange={(taskId, nextPosition) => {
|
||||
setPositions((current) => {
|
||||
const denormalizedPosition = { x: nextPosition.x + bounds.minX, y: nextPosition.y + bounds.minY };
|
||||
const existing = current.get(taskId);
|
||||
if (existing && existing.x === nextPosition.x && existing.y === nextPosition.y) return current;
|
||||
if (existing && existing.x === denormalizedPosition.x && existing.y === denormalizedPosition.y) return current;
|
||||
const next = new Map(current);
|
||||
next.set(taskId, nextPosition);
|
||||
next.set(taskId, denormalizedPosition);
|
||||
return next;
|
||||
});
|
||||
}}
|
||||
@@ -289,7 +317,14 @@ export function DependencyGraph({
|
||||
const viewport = viewportRef.current;
|
||||
if (!viewport) return;
|
||||
const freshLayout = computeAutoLayout(graphData, { nodeWidth: NODE_WIDTH, nodeHeight: NODE_HEIGHT, horizontalGap: 40, verticalGap: 80 });
|
||||
fitToGraph(freshLayout, viewport.clientWidth, viewport.clientHeight, { nodeWidth: NODE_WIDTH, nodeHeight: NODE_HEIGHT });
|
||||
const normalizedFreshLayout = new Map<string, { x: number; y: number }>();
|
||||
const values = Array.from(freshLayout.values());
|
||||
const minX = values.length > 0 ? Math.min(...values.map((position) => position.x)) : 0;
|
||||
const minY = values.length > 0 ? Math.min(...values.map((position) => position.y)) : 0;
|
||||
for (const [taskId, position] of freshLayout.entries()) {
|
||||
normalizedFreshLayout.set(taskId, { x: position.x - minX, y: position.y - minY });
|
||||
}
|
||||
fitToGraph(normalizedFreshLayout, viewport.clientWidth, viewport.clientHeight, { nodeWidth: NODE_WIDTH, nodeHeight: NODE_HEIGHT });
|
||||
}}
|
||||
onResetView={() => {
|
||||
handleResetLayout();
|
||||
|
||||
@@ -23,6 +23,7 @@ vi.mock("../useGraphInteraction", () => ({
|
||||
onPointerUp: vi.fn(),
|
||||
onWheelZoom: vi.fn(),
|
||||
handleKeyDown: vi.fn(),
|
||||
setGraphBounds: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
|
||||
@@ -25,6 +25,7 @@ vi.mock("../useGraphInteraction", () => ({
|
||||
onPointerUp: vi.fn(),
|
||||
onWheelZoom: vi.fn(),
|
||||
handleKeyDown: vi.fn(),
|
||||
setGraphBounds: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
@@ -80,8 +81,8 @@ describe("DependencyGraph persistence", () => {
|
||||
unmount();
|
||||
render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 20px");
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("top: 30px");
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 0px");
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("top: 0px");
|
||||
});
|
||||
|
||||
it("merges saved positions with auto-layout for new tasks", () => {
|
||||
@@ -89,8 +90,8 @@ describe("DependencyGraph persistence", () => {
|
||||
|
||||
render(<DependencyGraph tasks={[createTask("A"), createTask("B")]} projectId="p1" />);
|
||||
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 25px");
|
||||
expect(screen.getByTestId("graph-task-node-B").getAttribute("style")).toContain("left: 200px");
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 0px");
|
||||
expect(screen.getByTestId("graph-task-node-B").getAttribute("style")).toContain("left: 175px");
|
||||
});
|
||||
|
||||
it("fit to graph clears saved positions and reapplies auto-layout", () => {
|
||||
@@ -108,9 +109,9 @@ describe("DependencyGraph persistence", () => {
|
||||
window.localStorage.setItem("kb:p2:fusion-plugin-dependency-graph:positions", JSON.stringify({ A: { x: 33, y: 44 } }));
|
||||
|
||||
const { rerender } = render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 11px");
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 0px");
|
||||
|
||||
rerender(<DependencyGraph tasks={[createTask("A")]} projectId="p2" />);
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 33px");
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 0px");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,6 +11,7 @@ const handleKeyDown = vi.fn();
|
||||
const onPointerDown = vi.fn();
|
||||
const onPointerMove = vi.fn();
|
||||
const onPointerUp = vi.fn();
|
||||
const setGraphBounds = vi.fn();
|
||||
|
||||
vi.mock("@fusion/dashboard/app/components/TaskCard", () => ({
|
||||
TaskCard: ({ task, onOpenDetail, disableDrag }: { task: Task; onOpenDetail: (task: Task) => void; disableDrag?: boolean }) => (
|
||||
@@ -32,6 +33,7 @@ vi.mock("../useGraphInteraction", () => ({
|
||||
onPointerUp,
|
||||
onWheelZoom: vi.fn(),
|
||||
handleKeyDown,
|
||||
setGraphBounds,
|
||||
}),
|
||||
}));
|
||||
|
||||
@@ -49,6 +51,7 @@ describe("DependencyGraph", () => {
|
||||
onPointerDown.mockReset();
|
||||
onPointerMove.mockReset();
|
||||
onPointerUp.mockReset();
|
||||
setGraphBounds.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -86,6 +89,7 @@ describe("DependencyGraph", () => {
|
||||
it("auto-fits on initial load with active tasks", () => {
|
||||
render(<DependencyGraph tasks={[createTask("A", "todo")]} onOpenTaskDetail={vi.fn()} />);
|
||||
expect(fitToGraph).toHaveBeenCalled();
|
||||
expect(setGraphBounds).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("forwards keyboard events to interaction hook", () => {
|
||||
|
||||
@@ -131,4 +131,20 @@ describe("dependency graph interactions", () => {
|
||||
expect(screen.getByTestId("graph-task-node-A").className).toContain("graph-task-node--highlighted");
|
||||
expect(screen.getByTestId("graph-task-node-D").className).toContain("graph-task-node--dimmed");
|
||||
});
|
||||
|
||||
it("keeps far graph content reachable after zoom-in pan", () => {
|
||||
const { result } = renderHook(() => useGraphInteraction());
|
||||
|
||||
act(() => {
|
||||
result.current.setGraphBounds({ minX: 0, minY: 0, maxX: 2600, maxY: 1400 });
|
||||
result.current.onWheelZoom(-120, { x: 400, y: 300 }, 800, 600);
|
||||
result.current.onPointerDown(1, { x: 350, y: 250 });
|
||||
result.current.onPointerMove(1, { x: -1200, y: 250 }, 800, 600);
|
||||
result.current.onPointerUp(1);
|
||||
});
|
||||
|
||||
const minPanX = 800 - 2600 * result.current.zoom;
|
||||
expect(result.current.pan.x).toBeGreaterThanOrEqual(minPanX);
|
||||
expect(result.current.pan.x).toBeLessThanOrEqual(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -179,4 +179,58 @@ describe("useGraphInteraction", () => {
|
||||
expect(result.current.zoom).toBe(1);
|
||||
expect(result.current.pan).toEqual({ x: 0, y: 0 });
|
||||
});
|
||||
|
||||
it("allows panning across full graph width when zoomed to 2x", () => {
|
||||
const { result } = renderHook(() => useGraphInteraction());
|
||||
|
||||
act(() => {
|
||||
result.current.setGraphBounds({ minX: 0, minY: 0, maxX: 2000, maxY: 1200 });
|
||||
for (let i = 0; i < 10; i += 1) {
|
||||
result.current.zoomIn();
|
||||
}
|
||||
result.current.onPointerDown(1, { x: 200, y: 200 });
|
||||
result.current.onPointerMove(1, { x: 1200, y: 200 }, 800, 600);
|
||||
result.current.onPointerMove(1, { x: -2200, y: 200 }, 800, 600);
|
||||
result.current.onPointerUp(1);
|
||||
});
|
||||
|
||||
expect(result.current.zoom).toBeGreaterThanOrEqual(2);
|
||||
expect(result.current.pan.x).toBeGreaterThanOrEqual(800 - 2000 * result.current.zoom);
|
||||
expect(result.current.pan.x).toBeLessThanOrEqual(0);
|
||||
});
|
||||
|
||||
it("uses zoom-aware pan limits at max zoom", () => {
|
||||
const { result } = renderHook(() => useGraphInteraction());
|
||||
|
||||
act(() => {
|
||||
result.current.setGraphBounds({ minX: 0, minY: 0, maxX: 2000, maxY: 1200 });
|
||||
for (let i = 0; i < 30; i += 1) {
|
||||
result.current.zoomIn();
|
||||
}
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.onPointerDown(1, { x: 300, y: 200 });
|
||||
result.current.onPointerMove(1, { x: -6000, y: 200 }, 800, 600);
|
||||
result.current.onPointerUp(1);
|
||||
});
|
||||
|
||||
expect(result.current.zoom).toBe(3);
|
||||
expect(result.current.pan.x).toBe(800 - 2000 * 3);
|
||||
});
|
||||
|
||||
it("fits graphs with negative coordinates using full min/max bounds", () => {
|
||||
const { result } = renderHook(() => useGraphInteraction());
|
||||
|
||||
act(() => {
|
||||
result.current.fitToGraph(new Map([
|
||||
["A", { x: -500, y: -200 }],
|
||||
["B", { x: 900, y: 500 }],
|
||||
]), 900, 700, { nodeWidth: 280, nodeHeight: 100 });
|
||||
});
|
||||
|
||||
expect(result.current.zoom).toBeGreaterThan(0.1);
|
||||
expect(result.current.pan.x).toBeLessThanOrEqual(550);
|
||||
expect(result.current.pan.y).toBeLessThanOrEqual(260);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,6 +20,13 @@ interface PinchState {
|
||||
midpoint: PointerPoint;
|
||||
}
|
||||
|
||||
interface GraphBounds {
|
||||
minX: number;
|
||||
minY: number;
|
||||
maxX: number;
|
||||
maxY: number;
|
||||
}
|
||||
|
||||
function clamp(value: number, min: number, max: number): number {
|
||||
return Math.min(max, Math.max(min, value));
|
||||
}
|
||||
@@ -41,6 +48,7 @@ export function useGraphInteraction() {
|
||||
const dragStateRef = useRef<{ start: PointerPoint; panStart: PointerPoint } | null>(null);
|
||||
const pointersRef = useRef<Map<number, PointerPoint>>(new Map());
|
||||
const pinchRef = useRef<PinchState | null>(null);
|
||||
const graphBoundsRef = useRef<GraphBounds>({ minX: 0, minY: 0, maxX: 0, maxY: 0 });
|
||||
|
||||
useEffect(() => {
|
||||
panRef.current = pan;
|
||||
@@ -79,10 +87,26 @@ export function useGraphInteraction() {
|
||||
}, TRANSITION_TIMEOUT_MS);
|
||||
}, []);
|
||||
|
||||
const clampPan = useCallback((nextPan: PointerPoint, viewportWidth: number, viewportHeight: number) => ({
|
||||
x: clamp(nextPan.x, -viewportWidth, viewportWidth),
|
||||
y: clamp(nextPan.y, -viewportHeight, viewportHeight),
|
||||
}), []);
|
||||
const clampPan = useCallback((nextPan: PointerPoint, viewportWidth: number, viewportHeight: number, nextZoom = zoomRef.current) => {
|
||||
const bounds = graphBoundsRef.current;
|
||||
|
||||
if (bounds.maxX <= bounds.minX || bounds.maxY <= bounds.minY) {
|
||||
return {
|
||||
x: clamp(nextPan.x, -viewportWidth, viewportWidth),
|
||||
y: clamp(nextPan.y, -viewportHeight, viewportHeight),
|
||||
};
|
||||
}
|
||||
|
||||
const minPanX = viewportWidth - bounds.maxX * nextZoom;
|
||||
const maxPanX = -bounds.minX * nextZoom;
|
||||
const minPanY = viewportHeight - bounds.maxY * nextZoom;
|
||||
const maxPanY = -bounds.minY * nextZoom;
|
||||
|
||||
const clampedX = minPanX > maxPanX ? (minPanX + maxPanX) / 2 : clamp(nextPan.x, minPanX, maxPanX);
|
||||
const clampedY = minPanY > maxPanY ? (minPanY + maxPanY) / 2 : clamp(nextPan.y, minPanY, maxPanY);
|
||||
|
||||
return { x: clampedX, y: clampedY };
|
||||
}, []);
|
||||
|
||||
const zoomAtPoint = useCallback((
|
||||
nextZoomRaw: number,
|
||||
@@ -98,7 +122,7 @@ export function useGraphInteraction() {
|
||||
const nextPan = clampPan({
|
||||
x: anchor.x - (anchor.x - currentPan.x) * scaleRatio,
|
||||
y: anchor.y - (anchor.y - currentPan.y) * scaleRatio,
|
||||
}, viewportWidth, viewportHeight);
|
||||
}, viewportWidth, viewportHeight, nextZoom);
|
||||
|
||||
setZoom(nextZoom);
|
||||
setPan(nextPan);
|
||||
@@ -164,7 +188,7 @@ export function useGraphInteraction() {
|
||||
const panY = (viewportHeight - graphHeight * nextZoom) / 2 - minY * nextZoom;
|
||||
|
||||
setZoom(nextZoom);
|
||||
setPan(clampPan({ x: panX, y: panY }, viewportWidth, viewportHeight));
|
||||
setPan(clampPan({ x: panX, y: panY }, viewportWidth, viewportHeight, nextZoom));
|
||||
}, [clampPan, setAnimate]);
|
||||
|
||||
const onPointerDown = useCallback((pointerId: number, point: PointerPoint) => {
|
||||
@@ -241,6 +265,10 @@ export function useGraphInteraction() {
|
||||
zoomAtPoint(zoomRef.current * factor, point, viewportWidth, viewportHeight);
|
||||
}, [setAnimate, zoomAtPoint]);
|
||||
|
||||
const setGraphBounds = useCallback((bounds: GraphBounds) => {
|
||||
graphBoundsRef.current = bounds;
|
||||
}, []);
|
||||
|
||||
const handleKeyDown = useCallback((
|
||||
event: ReactKeyboardEvent,
|
||||
viewportWidth: number,
|
||||
@@ -299,5 +327,6 @@ export function useGraphInteraction() {
|
||||
onPointerUp,
|
||||
onWheelZoom,
|
||||
handleKeyDown,
|
||||
setGraphBounds,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user