NodeGraph: Make grid view responsive (#35047)

This commit is contained in:
Andrej Ocenas 2021-06-01 17:30:47 +02:00 committed by GitHub
parent e0a3f669e5
commit aa7dbd7a69
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 23 deletions

View File

@ -23,6 +23,15 @@ jest.mock('./layout.worker.js', () => {
};
});
jest.mock('react-use/lib/useMeasure', () => {
return {
__esModule: true,
default: () => {
return [() => {}, { width: 500, height: 200 }];
},
};
});
describe('NodeGraph', () => {
it('doesnt fail without any data', async () => {
render(<NodeGraph dataFrames={[]} getLinks={() => []} />);
@ -57,7 +66,6 @@ describe('NodeGraph', () => {
await screen.findByLabelText('Node: service:1');
panView({ x: 10, y: 10 });
screen.debug(getSvg());
// Though we try to pan down 10px we are rendering in straight line 3 nodes so there are bounds preventing
// as panning vertically
await waitFor(() => expect(getTranslate()).toEqual({ x: 10, y: 0 }));
@ -209,9 +217,9 @@ describe('NodeGraph', () => {
const button = await screen.findByTitle(/Grid layout/);
userEvent.click(button);
await expectNodePositionCloseTo('service:0', { x: -180, y: -60 });
await expectNodePositionCloseTo('service:1', { x: -60, y: -60 });
await expectNodePositionCloseTo('service:2', { x: 60, y: -60 });
await expectNodePositionCloseTo('service:0', { x: -60, y: -60 });
await expectNodePositionCloseTo('service:1', { x: 60, y: -60 });
await expectNodePositionCloseTo('service:2', { x: -60, y: 80 });
});
});

View File

@ -135,6 +135,7 @@ export function NodeGraph({ getLinks, dataFrames, nodeLimit }: Props) {
processed.edges,
config,
nodeCountLimit,
width,
focusedNodeId
);

View File

@ -44,11 +44,9 @@ export function useLayout(
rawEdges: EdgeDatum[],
config: Config = defaultConfig,
nodeCountLimit: number,
width: number,
rootNodeId?: string
) {
const [nodesGrid, setNodesGrid] = useState<NodeDatum[]>([]);
const [edgesGrid, setEdgesGrid] = useState<EdgeDatumLayout[]>([]);
const [nodesGraph, setNodesGraph] = useState<NodeDatum[]>([]);
const [edgesGraph, setEdgesGraph] = useState<EdgeDatumLayout[]>([]);
@ -70,18 +68,16 @@ export function useLayout(
// happening as already visible nodes would change positions.
useEffect(() => {
if (rawNodes.length === 0) {
setNodesGraph([]);
setEdgesGraph([]);
return;
}
setLoading(true);
// d3 just modifies the nodes directly, so lets make sure we don't leak that outside
let rawNodesCopy = rawNodes.map((n) => ({ ...n }));
let rawEdgesCopy = rawEdges.map((e) => ({ ...e }));
// This is async but as I wanted to still run the sync grid layout and you cannot return promise from effect having
// callback seem ok here.
defaultLayout(rawNodesCopy, rawEdgesCopy, ({ nodes, edges }) => {
// 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.
defaultLayout(rawNodes, rawEdges, ({ nodes, edges }) => {
// TODO: it would be better to cancel the worker somehow but probably not super important right now.
if (isMounted()) {
setNodesGraph(nodes);
@ -89,14 +85,21 @@ export function useLayout(
setLoading(false);
}
});
}, [rawNodes, rawEdges, isMounted]);
rawNodesCopy = rawNodes.map((n) => ({ ...n }));
rawEdgesCopy = rawEdges.map((e) => ({ ...e }));
gridLayout(rawNodesCopy, config.sort);
// 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.
const [nodesGrid, edgesGrid] = useMemo(() => {
if (rawNodes.length === 0) {
return [[], []];
}
setNodesGrid(rawNodesCopy);
setEdgesGrid(rawEdgesCopy as EdgeDatumLayout[]);
}, [config.sort, rawNodes, rawEdges, isMounted]);
const rawNodesCopy = rawNodes.map((n) => ({ ...n }));
const rawEdgesCopy = rawEdges.map((e) => ({ ...e }));
gridLayout(rawNodesCopy, width, config.sort);
return [rawNodesCopy, rawEdgesCopy as EdgeDatumLayout[]];
}, [config.sort, rawNodes, rawEdges, width]);
// Limit the nodes so we don't show all for performance reasons. Here we don't compute both at the same time so
// changing the layout can trash internal memoization at the moment.
@ -159,6 +162,7 @@ function defaultLayout(
*/
function gridLayout(
nodes: NodeDatum[],
width: number,
sort?: {
field: Field;
ascending: boolean;
@ -166,8 +170,9 @@ function gridLayout(
) {
const spacingVertical = 140;
const spacingHorizontal = 120;
// TODO probably make this based on the width of the screen
const perRow = 4;
const padding = spacingHorizontal / 2;
const perRow = Math.min(Math.floor((width - padding * 2) / spacingVertical), nodes.length);
const midPoint = Math.floor(((perRow - 1) * spacingHorizontal) / 2);
if (sort) {
nodes.sort((node1, node2) => {
@ -182,7 +187,7 @@ function gridLayout(
for (const [index, node] of nodes.entries()) {
const row = Math.floor(index / perRow);
const column = index % perRow;
node.x = -180 + column * spacingHorizontal;
node.x = column * spacingHorizontal - midPoint;
node.y = -60 + row * spacingVertical;
}
}