NodeGraph: Use values from fixedX/fixedY column for layout (#86643)

* NodeGraph: Use values from fixedX/fixedY column for layout

* Cleanup some code, typings and naming

* Add test

---------

Co-authored-by: Andrej Ocenas <mr.ocenas@gmail.com>
This commit is contained in:
timo 2024-05-28 15:34:35 +02:00 committed by GitHub
parent a738cb42d8
commit 6205236f25
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 122 additions and 33 deletions

View File

@ -4624,8 +4624,7 @@ exports[`better eslint`] = {
"public/app/plugins/panel/nodeGraph/EdgeLabel.tsx:5381": [
[0, 0, 0, "Styles should be written using objects.", "0"],
[0, 0, 0, "Styles should be written using objects.", "1"],
[0, 0, 0, "Styles should be written using objects.", "2"],
[0, 0, 0, "Do not use any type assertions.", "3"]
[0, 0, 0, "Styles should be written using objects.", "2"]
],
"public/app/plugins/panel/nodeGraph/Legend.tsx:5381": [
[0, 0, 0, "Styles should be written using objects.", "0"],
@ -4676,8 +4675,7 @@ exports[`better eslint`] = {
[0, 0, 0, "Do not re-export imported variable (\`./NodeGraph\`)", "0"]
],
"public/app/plugins/panel/nodeGraph/layout.ts:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"],
[0, 0, 0, "Do not use any type assertions.", "1"]
[0, 0, 0, "Do not use any type assertions.", "0"]
],
"public/app/plugins/panel/nodeGraph/types.ts:5381": [
[0, 0, 0, "Do not re-export imported variable (\`./panelcfg.gen\`)", "0"]

View File

@ -38,4 +38,10 @@ export enum NodeGraphDataFrameFieldNames {
// Defines the stroke dash array for the edge [edges]. See SVG strokeDasharray definition for syntax.
strokeDasharray = 'strokedasharray',
// Supplies a fixed X position for the node to have in the finished graph.
fixedX = 'fixedx',
// Supplies a fixed Y position for the node to have in the finished graph.
fixedY = 'fixedy',
}

View File

@ -2,17 +2,17 @@ import React, { MouseEvent, memo } from 'react';
import { EdgeArrowMarker } from './EdgeArrowMarker';
import { computeNodeCircumferenceStrokeWidth, nodeR } from './Node';
import { EdgeDatum, NodeDatum } from './types';
import { EdgeDatumLayout, NodeDatum } from './types';
import { shortenLine } from './utils';
export const defaultHighlightedEdgeColor = '#a00';
export const defaultEdgeColor = '#999';
interface Props {
edge: EdgeDatum;
edge: EdgeDatumLayout;
hovering: boolean;
svgIdNamespace: string;
onClick: (event: MouseEvent<SVGElement>, link: EdgeDatum) => void;
onClick: (event: MouseEvent<SVGElement>, link: EdgeDatumLayout) => void;
onMouseEnter: (id: string) => void;
onMouseLeave: (id: string) => void;
}

View File

@ -5,7 +5,7 @@ import { GrafanaTheme2 } from '@grafana/data';
import { useStyles2 } from '@grafana/ui';
import { nodeR } from './Node';
import { EdgeDatum, NodeDatum } from './types';
import { EdgeDatumLayout } from './types';
import { shortenLine } from './utils';
const getStyles = (theme: GrafanaTheme2) => {
@ -26,17 +26,12 @@ const getStyles = (theme: GrafanaTheme2) => {
};
interface Props {
edge: EdgeDatum;
edge: EdgeDatumLayout;
}
export const EdgeLabel = memo(function EdgeLabel(props: Props) {
const { edge } = props;
// Not great typing, but after we do layout these properties are full objects not just references
const { source, target, sourceNodeRadius, targetNodeRadius } = edge as {
source: NodeDatum;
target: NodeDatum;
sourceNodeRadius: number;
targetNodeRadius: number;
};
const { source, target, sourceNodeRadius, targetNodeRadius } = edge;
// As the nodes have some radius we want edges to end outside the node circle.
const line = shortenLine(

View File

@ -13,7 +13,7 @@ import { Marker } from './Marker';
import { Node } from './Node';
import { ViewControls } from './ViewControls';
import { Config, defaultConfig, useLayout } from './layout';
import { EdgeDatum, NodeDatum, NodesMarker } from './types';
import { EdgeDatumLayout, NodeDatum, NodesMarker } from './types';
import { useCategorizeFrames } from './useCategorizeFrames';
import { useContextMenu } from './useContextMenu';
import { useFocusPositionOnLayout } from './useFocusPositionOnLayout';
@ -164,7 +164,8 @@ export function NodeGraph({ getLinks, dataFrames, nodeLimit, panelId }: Props) {
config,
nodeCountLimit,
width,
focusedNodeId
focusedNodeId,
processed.hasFixedPositions
);
// If we move from grid to graph layout, and we have focused node lets get its position to center there. We want to
@ -338,11 +339,11 @@ const Markers = memo(function Nodes(props: MarkersProps) {
});
interface EdgesProps {
edges: EdgeDatum[];
edges: EdgeDatumLayout[];
nodeHoveringId?: string;
edgeHoveringId?: string;
svgIdNamespace: string;
onClick: (event: MouseEvent<SVGElement>, link: EdgeDatum) => void;
onClick: (event: MouseEvent<SVGElement>, link: EdgeDatumLayout) => void;
onMouseEnter: (id: string) => void;
onMouseLeave: (id: string) => void;
}
@ -369,7 +370,7 @@ const Edges = memo(function Edges(props: EdgesProps) {
});
interface EdgeLabelsProps {
edges: EdgeDatum[];
edges: EdgeDatumLayout[];
nodeHoveringId?: string;
edgeHoveringId?: string;
}

View File

@ -48,7 +48,8 @@ export function useLayout(
config: Config = defaultConfig,
nodeCountLimit: number,
width: number,
rootNodeId?: string
rootNodeId?: string,
hasFixedPositions?: boolean
) {
const [nodesGraph, setNodesGraph] = useState<NodeDatum[]>([]);
const [edgesGraph, setEdgesGraph] = useState<EdgeDatumLayout[]>([]);
@ -84,19 +85,36 @@ export function useLayout(
return;
}
if (hasFixedPositions) {
setNodesGraph(rawNodes);
// The layout function turns source and target fields from string to NodeDatum, so we do that here as well.
const nodesMap = fromPairs(rawNodes.map((node) => [node.id, node]));
setEdgesGraph(
rawEdges.map(
(e): EdgeDatumLayout => ({
...e,
source: nodesMap[e.source],
target: nodesMap[e.target],
})
)
);
setLoading(false);
return;
}
setLoading(true);
// This is async but as I wanted to still run the sync grid layout, and you cannot return promise from effect so
// having callback seems ok here.
const cancel = layout(rawNodes, rawEdges, ({ nodes, edges }) => {
if (isMounted()) {
setNodesGraph(nodes);
setEdgesGraph(edges as EdgeDatumLayout[]);
setEdgesGraph(edges);
setLoading(false);
}
});
layoutWorkerCancelRef.current = cancel;
return cancel;
}, [rawNodes, rawEdges, isMounted]);
}, [hasFixedPositions, rawNodes, rawEdges, isMounted]);
// Compute grid separately as it is sync and do not need to be inside effect. Also it is dependant on width while
// default layout does not care and we don't want to recalculate that on panel resize.
@ -149,7 +167,7 @@ export function useLayout(
function layout(
nodes: NodeDatum[],
edges: EdgeDatum[],
done: (data: { nodes: NodeDatum[]; edges: EdgeDatum[] }) => void
done: (data: { nodes: NodeDatum[]; edges: EdgeDatumLayout[] }) => void
) {
// const worker = engine === 'default' ? createWorker() : createMsaglWorker();
// TODO: temp fix because of problem with msagl library https://github.com/grafana/grafana/issues/83318

View File

@ -45,7 +45,7 @@ export type EdgeDatum = LinkDatum & {
};
// After layout is run D3 will change the string IDs for actual references to the nodes.
export type EdgeDatumLayout = EdgeDatum & {
export type EdgeDatumLayout = Omit<EdgeDatum, 'source' | 'target'> & {
source: NodeDatum;
target: NodeDatum;
};

View File

@ -5,7 +5,7 @@ import { DataFrame, Field, GrafanaTheme2, LinkModel } from '@grafana/data';
import { ContextMenu, MenuGroup, MenuItem, useStyles2 } from '@grafana/ui';
import { Config } from './layout';
import { EdgeDatum, NodeDatum } from './types';
import { EdgeDatumLayout, NodeDatum } from './types';
import { getEdgeFields, getNodeFields, statToString } from './utils';
/**
@ -22,7 +22,7 @@ export function useContextMenu(
setConfig: (config: Config) => void,
setFocusedNodeId: (id: string) => void
): {
onEdgeOpen: (event: MouseEvent<SVGElement>, edge: EdgeDatum) => void;
onEdgeOpen: (event: MouseEvent<SVGElement>, edge: EdgeDatumLayout) => void;
onNodeOpen: (event: MouseEvent<SVGElement>, node: NodeDatum) => void;
MenuComponent: React.ReactNode;
} {
@ -53,7 +53,7 @@ export function useContextMenu(
);
const onEdgeOpen = useCallback(
(event: MouseEvent<SVGElement>, edge: EdgeDatum) => {
(event: MouseEvent<SVGElement>, edge: EdgeDatumLayout) => {
if (!edges) {
// This could happen if we have only one node and no edges, in which case this is not needed as there is no edge
// to click on.
@ -86,7 +86,7 @@ function makeContextMenu(
);
}
function getItemsRenderer<T extends NodeDatum | EdgeDatum>(
function getItemsRenderer<T extends NodeDatum | EdgeDatumLayout>(
links: LinkModel[],
item: T,
extraItems?: Array<LinkData<T>> | undefined
@ -109,7 +109,7 @@ function getItemsRenderer<T extends NodeDatum | EdgeDatum>(
};
}
function mapMenuItem<T extends NodeDatum | EdgeDatum>(item: T) {
function mapMenuItem<T extends NodeDatum | EdgeDatumLayout>(item: T) {
return function NodeGraphMenuItem(link: LinkData<T>) {
return (
<MenuItem
@ -134,7 +134,7 @@ function mapMenuItem<T extends NodeDatum | EdgeDatum>(item: T) {
};
}
type LinkData<T extends NodeDatum | EdgeDatum> = {
type LinkData<T extends NodeDatum | EdgeDatumLayout> = {
label: string;
ariaLabel?: string;
url?: string;
@ -224,7 +224,7 @@ function NodeHeader({ node, nodes }: { node: NodeDatum; nodes?: DataFrame }) {
/**
* Shows some of the field values in a table on top of the context menu.
*/
function EdgeHeader(props: { edge: EdgeDatum; edges: DataFrame }) {
function EdgeHeader(props: { edge: EdgeDatumLayout; edges: DataFrame }) {
const index = props.edge.dataFrameRowIndex;
const fields = getEdgeFields(props.edges);
const valueSource = fields.source?.values[index] || '';

View File

@ -1,4 +1,4 @@
import { DataFrame, FieldType, createDataFrame } from '@grafana/data';
import { DataFrame, FieldType, createDataFrame, NodeGraphDataFrameFieldNames } from '@grafana/data';
import { NodeDatum, NodeGraphOptions } from './types';
import {
@ -224,6 +224,49 @@ describe('processNodes', () => {
expect(edgesFrame?.fields.find((f) => f.name === 'mainStat')?.config).toEqual({ unit: 'r/sec' });
expect(edgesFrame?.fields.find((f) => f.name === 'secondaryStat')?.config).toEqual({ unit: 'ft^2' });
});
it('processes nodes with fixedX/Y', async () => {
const nodesFrame = makeNodesDataFrame(3);
nodesFrame.fields.push({
name: NodeGraphDataFrameFieldNames.fixedX,
type: FieldType.number,
values: [1, 2, 3],
config: {},
});
nodesFrame.fields.push({
name: NodeGraphDataFrameFieldNames.fixedY,
type: FieldType.number,
values: [1, 2, 3],
config: {},
});
const result = processNodes(nodesFrame, undefined);
expect(result.hasFixedPositions).toBe(true);
expect(result.nodes[0].x).toBe(1);
expect(result.nodes[0].y).toBe(1);
});
it('throws error if fixedX/Y is used incorrectly', async () => {
const nodesFrame = makeNodesDataFrame(3);
nodesFrame.fields.push({
name: NodeGraphDataFrameFieldNames.fixedX,
type: FieldType.number,
values: [undefined, 2, 3],
config: {},
});
expect(() => processNodes(nodesFrame, undefined)).toThrow(/fixedX/);
// We still have one undefined value in fixedX field so this should still fail
nodesFrame.fields.push({
name: NodeGraphDataFrameFieldNames.fixedY,
type: FieldType.number,
values: [1, 2, 3],
config: {},
});
expect(() => processNodes(nodesFrame, undefined)).toThrow(/fixedX/);
});
});
describe('finds connections', () => {

View File

@ -45,6 +45,8 @@ export function shortenLine(line: Line, sourceNodeRadius: number, targetNodeRadi
}
export type NodeFields = {
fixedX?: Field;
fixedY?: Field;
id?: Field;
title?: Field;
subTitle?: Field;
@ -76,6 +78,8 @@ export function getNodeFields(nodes: DataFrame): NodeFields {
icon: fieldsCache.getFieldByName(NodeGraphDataFrameFieldNames.icon),
nodeRadius: fieldsCache.getFieldByName(NodeGraphDataFrameFieldNames.nodeRadius.toLowerCase()),
highlighted: fieldsCache.getFieldByName(NodeGraphDataFrameFieldNames.highlighted.toLowerCase()),
fixedX: fieldsCache.getFieldByName(NodeGraphDataFrameFieldNames.fixedX.toLowerCase()),
fixedY: fieldsCache.getFieldByName(NodeGraphDataFrameFieldNames.fixedY.toLowerCase()),
};
}
@ -129,6 +133,7 @@ export function processNodes(
): {
nodes: NodeDatum[];
edges: EdgeDatum[];
hasFixedPositions?: boolean;
legend?: Array<{
color: string;
name: string;
@ -144,6 +149,24 @@ export function processNodes(
throw new Error('id field is required for nodes data frame.');
}
const hasFixedPositions =
nodeFields.fixedX &&
nodeFields.fixedX.values.every((v) => Number.isFinite(v)) &&
nodeFields.fixedY &&
nodeFields.fixedY.values.every((v) => Number.isFinite(v));
// Throw an error if somebody is using fixedX and fixedY fields incorrectly. Other option is to ignore this but we
// are not able to easily combine fixed and non-fixed position in layout so that behaviour would be undefined
// and silent.
if (!hasFixedPositions) {
const somePosFilled =
(nodeFields.fixedX && nodeFields.fixedX.values.some((v) => Number.isFinite(v))) ||
(nodeFields.fixedY && nodeFields.fixedY.values.some((v) => Number.isFinite(v)));
if (somePosFilled) {
throw new Error('If fixedX and fixedY fields are present, the values have to be all filled and valid');
}
}
// Create the nodes here
const nodesMap: { [id: string]: NodeDatum } = {};
for (let i = 0; i < nodeFields.id.values.length; i++) {
@ -162,6 +185,7 @@ export function processNodes(
return {
nodes: Object.values(nodesMap),
edges: edgeDatums,
hasFixedPositions,
legend: nodeFields.arc.map((f) => {
return {
color: f.config.color?.fixedColor ?? '',
@ -210,6 +234,8 @@ export function processNodes(
return {
nodes,
edges: edgeDatums,
// Edge-only datasets never have fixedX/fixedY
hasFixedPositions: false,
};
}
}
@ -336,6 +362,8 @@ function makeNodeDatum(id: string, nodeFields: NodeFields, index: number): NodeD
icon: nodeFields.icon?.values[index] || '',
nodeRadius: nodeFields.nodeRadius,
highlighted: nodeFields.highlighted?.values[index] || false,
x: nodeFields.fixedX?.values[index] ?? undefined,
y: nodeFields.fixedY?.values[index] ?? undefined,
};
}