diff --git a/.changeset/bright-graphs-wrap.md b/.changeset/bright-graphs-wrap.md new file mode 100644 index 000000000..5cd86020c --- /dev/null +++ b/.changeset/bright-graphs-wrap.md @@ -0,0 +1,5 @@ +--- +"@preact/signals-devtools-ui": patch +--- + +Wrap dense dependency graph layers into columns so large application graphs remain visible when fitted to the viewport. diff --git a/packages/devtools-ui/src/components/Graph.tsx b/packages/devtools-ui/src/components/Graph.tsx index 89fdd3b19..4423b1015 100644 --- a/packages/devtools-ui/src/components/Graph.tsx +++ b/packages/devtools-ui/src/components/Graph.tsx @@ -12,6 +12,12 @@ const DEFAULT_VIEWPORT_SIZE = { width: 800, height: 600 }; const FIT_PADDING = 80; const MIN_ZOOM = 0.001; const MAX_ZOOM = 5; +const MIN_NODES_PER_COLUMN = 12; +const TARGET_GRAPH_ASPECT_RATIO = 1.6; +const NODE_COLUMN_SPACING = 160; +const NODE_ROW_SPACING = 120; +const LAYER_SPACING = 250; +const GRAPH_PADDING = 100; interface Point { x: number; @@ -436,28 +442,55 @@ export function GraphVisualization() { // Minimize edge crossings minimizeCrossings(nodesByLayer, allLinks); - // Layout nodes with proper spacing. Keep every layer within positive graph - // coordinates so tall layers are pannable instead of being clipped above 0. - const nodeSpacing = 120; - const layerSpacing = 250; - const graphPadding = 100; - const maxLayerNodeCount = Math.max( + // Dense layers used to form a single, extremely tall column. Fitting a large + // application graph then reduced every node to an indistinguishable vertical + // line. Wrap dense layers into columns sized to avoid pathological graph + // aspect ratios while preserving left-to-right depth order. + const nodesPerColumn = Math.max( + MIN_NODES_PER_COLUMN, + Math.ceil( + Math.sqrt( + (nodes.size * NODE_COLUMN_SPACING) / + (TARGET_GRAPH_ASPECT_RATIO * NODE_ROW_SPACING) + ) + ) + ); + const maxColumnNodeCount = Math.max( 1, - ...Array.from(nodesByLayer.values()).map(layerNodes => layerNodes.length) + ...Array.from(nodesByLayer.values()).map(layerNodes => + Math.min(layerNodes.length, nodesPerColumn) + ) ); const graphHeight = - graphPadding * 2 + Math.max(0, maxLayerNodeCount - 1) * nodeSpacing; + GRAPH_PADDING * 2 + + Math.max(0, maxColumnNodeCount - 1) * NODE_ROW_SPACING; + const graphInnerHeight = graphHeight - GRAPH_PADDING * 2; + const orderedLayers = Array.from(nodesByLayer.keys()).sort((a, b) => a - b); + let layerStartX = GRAPH_PADDING; - nodesByLayer.forEach((layerNodes, layer) => { - const layerHeight = (layerNodes.length - 1) * nodeSpacing; - const layerStartY = - graphPadding + (graphHeight - graphPadding * 2 - layerHeight) / 2; + for (const layer of orderedLayers) { + const layerNodes = nodesByLayer.get(layer)!; + const columnCount = Math.ceil(layerNodes.length / nodesPerColumn); layerNodes.forEach((node, index) => { - node.x = graphPadding + layer * layerSpacing; - node.y = layerStartY + index * nodeSpacing; + const column = Math.floor(index / nodesPerColumn); + const row = index % nodesPerColumn; + const columnNodeCount = Math.min( + nodesPerColumn, + layerNodes.length - column * nodesPerColumn + ); + const columnHeight = (columnNodeCount - 1) * NODE_ROW_SPACING; + + node.x = layerStartX + column * NODE_COLUMN_SPACING; + node.y = + GRAPH_PADDING + + (graphInnerHeight - columnHeight) / 2 + + row * NODE_ROW_SPACING; }); - }); + + layerStartX += + Math.max(0, columnCount - 1) * NODE_COLUMN_SPACING + LAYER_SPACING; + } const graphNodes = Array.from(nodes.values()); const nodeLookup = new Map(graphNodes.map(node => [node.id, node])); diff --git a/packages/devtools-ui/test/browser/index.test.tsx b/packages/devtools-ui/test/browser/index.test.tsx index ed5118ba9..19ff1591c 100644 --- a/packages/devtools-ui/test/browser/index.test.tsx +++ b/packages/devtools-ui/test/browser/index.test.tsx @@ -1125,6 +1125,76 @@ describe("@preact/signals-devtools-ui", () => { expect(links.length).to.equal(2); }); + it("should wrap dense layers instead of shrinking them into a line", () => { + initDevTools(mockAdapter); + + const denseLayerUpdates = Array.from({ length: 1000 }, (_, index) => ({ + type: "update" as const, + signalType: "signal" as const, + signalName: `signal-${index}`, + signalId: `signal-${index}`, + prevValue: index, + newValue: index + 1, + receivedAt: Date.now(), + })); + + mockAdapter._emit("signalUpdate", denseLayerUpdates); + render(, scratch); + + const nodes = Array.from( + scratch.querySelectorAll(".graph-node") + ); + const xPositions = new Set(nodes.map(node => node.getAttribute("cx"))); + const yPositions = new Set(nodes.map(node => node.getAttribute("cy"))); + const zoomPercent = parseInt( + scratch.querySelector(".graph-zoom-indicator")!.textContent!, + 10 + ); + + expect(nodes).to.have.length(1000); + expect(xPositions.size).to.be.greaterThan(10); + expect(yPositions.size).to.be.lessThan(50); + expect(zoomPercent).to.be.greaterThan(5); + }); + + it("should keep a wrapped dependency layer before its dependent layer", () => { + initDevTools(mockAdapter); + + const dependencies = Array.from({ length: 80 }, (_, index) => ({ + id: `signal-${index}`, + name: `signal-${index}`, + type: "signal" as const, + })); + mockAdapter._emit("signalUpdate", [ + { + type: "update", + signalType: "computed", + signalName: "total", + signalId: "computed-total", + prevValue: 0, + newValue: 1, + receivedAt: Date.now(), + allDependencies: dependencies, + }, + ]); + render(, scratch); + + const signals = Array.from( + scratch.querySelectorAll(".graph-node.signal") + ); + const computed = scratch.querySelector( + ".graph-node.computed" + )!; + const signalXPositions = signals.map(node => + Number(node.getAttribute("cx")) + ); + + expect(new Set(signalXPositions).size).to.be.greaterThan(1); + expect(Math.max(...signalXPositions)).to.be.lessThan( + Number(computed.getAttribute("cx")) + ); + }); + it("should export only graph nodes and links as JSON", async () => { initDevTools(mockAdapter);