diff --git a/frontend/src/components/Flow/layout.ts b/frontend/src/components/Flow/layout.ts index e93921b..ecce134 100644 --- a/frontend/src/components/Flow/layout.ts +++ b/frontend/src/components/Flow/layout.ts @@ -21,22 +21,44 @@ const NODE_H = 56 * Room for the live value an edge carries (`LiveEdge`'s chip is * `max-w-[140px]`). Reserved on the edge itself, so dagre routes nodes around * the chip rather than through it. + * + * Only its height is reserved when the graph runs downwards. The chip's width + * is then spent across the screen rather than along the flow, and a phone has + * none to spare — while the rank gap it would otherwise widen is already 80px, + * more than the chip is tall. Sibling edges are a node's width apart there, so + * the chips clear each other without being asked to. */ const LABEL_W = 150 const LABEL_H = 24 /** - * Lay the graph out and return each node's top-left corner. + * How many nodes may sit abreast when the graph runs downwards. * - * ponytail: every node is treated as 220×56 rather than measured. Measuring - * would feed the result back into the layout and oscillate; if nodes ever grow - * past that box, take the sizes from `node.measured` once they have settled. + * A node box is landscape, so siblings cost a rank four times as much across + * as they do down it: eight of them side by side is two thousand pixels, which + * a phone can only show by shrinking the labels out of existence. Two abreast + * is 480px — the fit still reads at 390 — and anything beyond that wraps onto + * the ranks below, so a wide fan-out grows the way the screen does. */ -export function layoutGraph( +const ABREAST = 2 + +/** + * How many times to let the wrapping settle before taking what it has. + * + * A bound rather than a fixed point: each pass moves nodes strictly downwards, + * so it does converge, but a pathological graph should not be allowed to + * relayout itself twenty times on a phone. + */ +const WRAP_PASSES = 6 + +type Wrap = [string, string] + +function build( ids: string[], edges: { source: string; target: string }[], direction: Direction, -): Map { + wrap: Wrap[], +) { const graph = new dagre.graphlib.Graph() graph.setDefaultEdgeLabel(() => ({})) graph.setGraph({ @@ -57,13 +79,72 @@ export function layoutGraph( for (const edge of edges) { if (!graph.hasNode(edge.source) || !graph.hasNode(edge.target)) continue graph.setEdge(edge.source, edge.target, { - width: LABEL_W, + width: direction === "LR" ? LABEL_W : 0, height: LABEL_H, labelpos: "c", }) } + // Nothing draws these: they only say "put this one a rank further down". + for (const [from, to] of wrap) graph.setEdge(from, to, { weight: 2 }) dagre.layout(graph) + return graph +} + +/** + * Which nodes have to move down a rank for the graph to stay narrow. + * + * Two nodes of the same rank never have an edge between them, so chaining the + * third to the first — and the fourth to the second, and so on — cannot make a + * cycle. It lays the rank out in rows of `ABREAST`, in the order the document + * lists them, so the wrap reads the way the flow is written. + */ +function wrapWideRanks(graph: ReturnType, ids: string[]): Wrap[] { + const ranks = new Map() + for (const id of ids) { + const node = graph.node(id) as { rank?: number } | undefined + if (node?.rank === undefined) continue + ranks.set(node.rank, [...(ranks.get(node.rank) ?? []), id]) + } + + const wrap: Wrap[] = [] + for (const rank of ranks.values()) { + if (rank.length <= ABREAST) continue + for (let i = ABREAST; i < rank.length; i += 1) { + wrap.push([rank[i - ABREAST], rank[i]]) + } + } + return wrap +} + +/** + * Lay the graph out and return each node's top-left corner. + * + * ponytail: every node is treated as 220×56 rather than measured. Measuring + * would feed the result back into the layout and oscillate; if nodes ever grow + * past that box, take the sizes from `node.measured` once they have settled. + */ +export function layoutGraph( + ids: string[], + edges: { source: string; target: string }[], + direction: Direction, +): Map { + let graph = build(ids, edges, direction, []) + + // Running downwards, a rank wider than the screen is the one thing the + // layout can still do something about. Wrapping one rank pushes whatever was + // below it up into the room that freed, which can overfill a rank that was + // fine — so it settles rather than being done once. A handful of passes over + // a graph of a few dozen nodes, and none at all when nothing is too wide. + if (direction === "TB") { + const wrap: Wrap[] = [] + for (let pass = 0; pass < WRAP_PASSES; pass += 1) { + const more = wrapWideRanks(graph, ids) + if (!more.length) break + wrap.push(...more) + graph = build(ids, edges, direction, wrap) + } + } // dagre places centres; React Flow wants top-left corners. return new Map( diff --git a/frontend/src/components/Health/HealthActivity.tsx b/frontend/src/components/Health/HealthActivity.tsx index f908deb..a409485 100644 --- a/frontend/src/components/Health/HealthActivity.tsx +++ b/frontend/src/components/Health/HealthActivity.tsx @@ -28,6 +28,18 @@ import { const RUNS_SHOWN = 15 const FAILURES_SHOWN = 25 +/** + * A list of rows in a card: capped and scrolled rather than as long as it + * happens to be. + * + * The cap is the visible reason; the scroll container is the structural one. A + * row is `min-w-0 flex-1 truncate`, and a truncating flex item still offers its + * whole unwrapped line as a min-content contribution — so without this the card + * sizes to the longest entry it holds and takes the page sideways with it. See + * DESIGN-GUIDELINES.md → Responsive. + */ +const LIST = "max-h-96 overflow-y-auto" + /** * A moment picked off a chart. * @@ -275,9 +287,7 @@ export function HealthActivity({ range }: { range: Range }) {

Given up on

-
+
{dead.map((item) => (

Changes

-
+
{audit?.length ? ( audit.map((event) => (
{ const page = await apiPage(browser) - // A chain and a fork, so the canvas has a graph worth laying out rather - // than a single node that fits anywhere. + // A chain and a four-way fan: enough of a graph to lay out, and wider than + // a phone can take abreast, so the wrapping is exercised. await api(page, `/flows/${flowName}`, { method: "PUT", data: { @@ -59,16 +59,11 @@ test.beforeAll(async ({ browser }) => { requires: [{ name: "reading", dtype: "float" }], provides: [{ name: "scaled", dtype: "float" }], }, - { - id: "left", + ...["one", "two", "three", "four"].map((id) => ({ + id, type: "python", requires: [{ name: "scaled", dtype: "float" }], - }, - { - id: "right", - type: "python", - requires: [{ name: "scaled", dtype: "float" }], - }, + })), ], version: 1, }, @@ -159,6 +154,18 @@ test("the flow editor fits, and its dock is reachable", async ({ page }) => { ) expect(Math.abs(scale.x - source.x)).toBeLessThan(200) + // …and it grows downwards rather than sideways: the four consumers of one + // message wrap onto rows instead of standing eight hundred pixels abreast. + const rows = new Map() + for (const id of ["source", "scale", "one", "two", "three", "four"]) { + const { y } = await nodeAt(page, id) + rows.set(y, (rows.get(y) ?? 0) + 1) + } + expect( + Math.max(...rows.values()), + "a row is wider than a phone", + ).toBeLessThanOrEqual(2) + // The dock used to overflow a phone, which put the buttons at its ends // outside the shell's `overflow-hidden` and made them unclickable. for (const id of ["add-node", "run-flow", "edit-flow", "publish-flow"]) {