Skip to content

Commit 4e0885f

Browse files
paula-stachopaula-stacho
authored
fix/COMPASS-9664 Do not require measured heights for relationships (#104)
1 parent c40fb28 commit 4e0885f

File tree

9 files changed

+187
-81
lines changed

9 files changed

+187
-81
lines changed

‎src/components/edge/floating-edge.test.tsx‎

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { Position, useNodes } from '@xyflow/react';
1+
import { Node, Position, useNodes } from '@xyflow/react';
22
import { ComponentProps } from 'react';
33

44
import { EMPLOYEES_NODE, ORDERS_NODE } from '@/mocks/datasets/nodes';
55
import { render, screen } from '@/mocks/testing-utils';
66
import { FloatingEdge } from '@/components/edge/floating-edge';
7+
import { DEFAULT_FIELD_HEIGHT, DEFAULT_NODE_WIDTH } from '@/utilities/constants';
78

89
vi.mock('@xyflow/react', async () => {
910
const actual = await vi.importActual<typeof import('@xyflow/react')>('@xyflow/react');
@@ -13,14 +14,23 @@ vi.mock('@xyflow/react', async () => {
1314
};
1415
});
1516

17+
function mockNodes(nodes: Node[]) {
18+
const mockedNodes = vi.mocked(useNodes);
19+
mockedNodes.mockReturnValue(nodes);
20+
}
21+
1622
describe('floating-edge', () => {
23+
const nodes = [
24+
{ ...ORDERS_NODE, data: { title: ORDERS_NODE.title, fields: ORDERS_NODE.fields } },
25+
{ ...EMPLOYEES_NODE, data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields } },
26+
];
27+
1728
beforeEach(() => {
18-
const nodes = [
19-
{ ...ORDERS_NODE, data: { title: ORDERS_NODE.title, fields: ORDERS_NODE.fields } },
20-
{ ...EMPLOYEES_NODE, data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields } },
21-
];
22-
const mockedNodes = vi.mocked(useNodes);
23-
mockedNodes.mockReturnValue(nodes);
29+
mockNodes(nodes);
30+
});
31+
32+
afterEach(() => {
33+
vi.clearAllMocks();
2434
});
2535

2636
const renderComponent = (props?: Partial<ComponentProps<typeof FloatingEdge>>) => {
@@ -40,14 +50,44 @@ describe('floating-edge', () => {
4050
);
4151
};
4252

43-
it('Should render edge', () => {
44-
renderComponent();
45-
const path = screen.getByTestId('floating-edge-orders-to-employees');
46-
expect(path).toHaveAttribute('id', 'orders-to-employees');
47-
expect(path).toHaveAttribute(
48-
'd',
49-
'M240 143.5L240 163.5L 240,213Q 240,218 245,218L 381,218Q 386,218 386,223L386 272.5L386 292.5',
50-
);
53+
describe('Without measured heights', () => {
54+
it('Should render edge', () => {
55+
renderComponent();
56+
const path = screen.getByTestId('floating-edge-orders-to-employees');
57+
expect(path).toHaveAttribute('id', 'orders-to-employees');
58+
expect(path).toHaveAttribute(
59+
'd',
60+
'M269.5 180L289.5 180L 386.5,180Q 391.5,180 391.5,185L 391.5,295Q 391.5,300 386.5,300L371.5 300',
61+
);
62+
});
63+
});
64+
65+
describe('With measured heights', () => {
66+
it('Should render edge', () => {
67+
mockNodes([
68+
{
69+
...nodes[0],
70+
measured: {
71+
width: DEFAULT_NODE_WIDTH,
72+
height: DEFAULT_FIELD_HEIGHT * 2,
73+
},
74+
},
75+
{
76+
...nodes[1],
77+
measured: {
78+
width: DEFAULT_NODE_WIDTH,
79+
height: DEFAULT_FIELD_HEIGHT * 4,
80+
},
81+
},
82+
]);
83+
renderComponent();
84+
const path = screen.getByTestId('floating-edge-orders-to-employees');
85+
expect(path).toHaveAttribute('id', 'orders-to-employees');
86+
expect(path).toHaveAttribute(
87+
'd',
88+
'M240 143.5L240 163.5L 240,213Q 240,218 245,218L 381,218Q 386,218 386,223L386 272.5L386 292.5',
89+
);
90+
});
5191
});
5292

5393
it('Should not render edge if source does not exist', () => {

‎src/components/edge/self-referencing-edge.test.tsx‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { Position, useNodes } from '@xyflow/react';
1+
import { Node, Position, useNodes } from '@xyflow/react';
22
import { ComponentProps } from 'react';
33

44
import { EMPLOYEES_NODE } from '@/mocks/datasets/nodes';
55
import { render, screen } from '@/mocks/testing-utils';
66
import { FloatingEdge } from '@/components/edge/floating-edge';
77
import { SelfReferencingEdge } from '@/components/edge/self-referencing-edge';
8+
import { DEFAULT_FIELD_HEIGHT, DEFAULT_NODE_WIDTH } from '@/utilities/constants';
89

910
vi.mock('@xyflow/react', async () => {
1011
const actual = await vi.importActual<typeof import('@xyflow/react')>('@xyflow/react');
@@ -14,11 +15,20 @@ vi.mock('@xyflow/react', async () => {
1415
};
1516
});
1617

18+
function mockNodes(nodes: Node[]) {
19+
const mockedNodes = vi.mocked(useNodes);
20+
mockedNodes.mockReturnValue(nodes);
21+
}
22+
1723
describe('self-referencing-edge', () => {
24+
const nodes = [{ ...EMPLOYEES_NODE, data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields } }];
25+
1826
beforeEach(() => {
19-
const nodes = [{ ...EMPLOYEES_NODE, data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields } }];
20-
const mockedNodes = vi.mocked(useNodes);
21-
mockedNodes.mockReturnValue(nodes);
27+
mockNodes(nodes);
28+
});
29+
30+
afterEach(() => {
31+
vi.clearAllMocks();
2232
});
2333

2434
const renderComponent = (props?: Partial<ComponentProps<typeof FloatingEdge>>) => {
@@ -38,11 +48,31 @@ describe('self-referencing-edge', () => {
3848
);
3949
};
4050

41-
it('Should render edge', () => {
42-
renderComponent();
43-
const path = screen.getByTestId('self-referencing-edge-employees-to-employees');
44-
expect(path).toHaveAttribute('id', 'employees-to-employees');
45-
expect(path).toHaveAttribute('d', 'M422,292.5L422,262.5L584,262.5L584,328.5L551.5,328.5');
51+
describe('Without measured heights', () => {
52+
it('Should render edge', () => {
53+
renderComponent();
54+
const path = screen.getByTestId('self-referencing-edge-employees-to-employees');
55+
expect(path).toHaveAttribute('id', 'employees-to-employees');
56+
expect(path).toHaveAttribute('d', 'M422,292.5L422,262.5L584,262.5L584,350.5L551.5,350.5');
57+
});
58+
});
59+
60+
describe('With measured heights', () => {
61+
it('Should render edge', () => {
62+
mockNodes([
63+
{
64+
...nodes[0],
65+
measured: {
66+
width: DEFAULT_NODE_WIDTH,
67+
height: DEFAULT_FIELD_HEIGHT * 4,
68+
},
69+
},
70+
]);
71+
renderComponent();
72+
const path = screen.getByTestId('self-referencing-edge-employees-to-employees');
73+
expect(path).toHaveAttribute('id', 'employees-to-employees');
74+
expect(path).toHaveAttribute('d', 'M422,292.5L422,262.5L584,262.5L584,328.5L551.5,328.5');
75+
});
4676
});
4777

4878
it('Should not render edge if source does not exist', () => {

‎src/components/edge/self-referencing-edge.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { path } from 'd3-path';
55
import { InternalNode } from '@/types/internal';
66
import { DEFAULT_MARKER_SIZE } from '@/utilities/constants';
77
import { Edge } from '@/components/edge/edge';
8+
import { getNodeWidth, getNodeHeight } from '@/utilities/node-dimensions';
89

910
export const SelfReferencingEdge = ({ id, source, markerEnd, markerStart, selected }: EdgeProps) => {
1011
const nodes = useNodes<InternalNode>();
@@ -18,8 +19,8 @@ export const SelfReferencingEdge = ({ id, source, markerEnd, markerStart, select
1819
return null;
1920
}
2021

21-
const centerX = (sourceNode.measured?.width || 0) / 2;
22-
const centerY = (sourceNode.measured?.height || 0) / 2;
22+
const centerX = getNodeWidth(sourceNode) / 2;
23+
const centerY = getNodeHeight(sourceNode) / 2;
2324

2425
const width = centerX + 40;
2526
const leftHeight = 30;

‎src/mocks/datasets/nodes.tsx‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { NodeProps } from '@/types';
2-
import { DEFAULT_FIELD_HEIGHT, DEFAULT_NODE_WIDTH } from '@/utilities/constants';
32

43
export const ORDERS_NODE: NodeProps = {
54
id: 'orders',
@@ -8,10 +7,6 @@ export const ORDERS_NODE: NodeProps = {
87
x: 100,
98
y: 100,
109
},
11-
measured: {
12-
width: DEFAULT_NODE_WIDTH,
13-
height: DEFAULT_FIELD_HEIGHT * 2,
14-
},
1510
title: 'orders',
1611
fields: [
1712
{ name: 'ORDER_ID', type: 'varchar', glyphs: ['key'] },
@@ -26,10 +21,6 @@ export const EMPLOYEES_NODE: NodeProps = {
2621
x: 300,
2722
y: 300,
2823
},
29-
measured: {
30-
width: DEFAULT_NODE_WIDTH,
31-
height: DEFAULT_FIELD_HEIGHT * 4,
32-
},
3324
title: 'employees',
3425
fields: [
3526
{ name: 'employeeId', type: 'objectId', glyphs: ['key'] },
@@ -46,10 +37,6 @@ export const EMPLOYEE_TERRITORIES_NODE: NodeProps = {
4637
x: 400,
4738
y: 100,
4839
},
49-
measured: {
50-
width: DEFAULT_NODE_WIDTH,
51-
height: DEFAULT_FIELD_HEIGHT * 4,
52-
},
5340
title: 'employee_territories',
5441
fields: [
5542
{ name: 'employeeId', type: 'string', glyphs: ['key'] },

‎src/utilities/apply-layout.ts‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
import { ElkExtendedEdge } from 'elkjs';
22
import ELK from 'elkjs/lib/elk.bundled';
33

4-
import {
5-
DEFAULT_FIELD_HEIGHT,
6-
DEFAULT_FIELD_PADDING,
7-
DEFAULT_NODE_HEADER_HEIGHT,
8-
DEFAULT_NODE_SPACING,
9-
DEFAULT_NODE_STAR_SPACING,
10-
DEFAULT_NODE_WIDTH,
11-
} from '@/utilities/constants';
4+
import { DEFAULT_NODE_SPACING, DEFAULT_NODE_STAR_SPACING } from '@/utilities/constants';
125
import { ApplyLayout, BaseEdge, BaseNode, LayoutDirection } from '@/types/layout';
136

7+
import { getNodeHeight, getNodeWidth } from './node-dimensions';
8+
149
const TOP_BOTTOM = {
1510
'elk.algorithm': 'layered',
1611
'elk.direction': 'UP',
@@ -42,11 +37,6 @@ const getLayoutOptions = (direction: LayoutDirection) => {
4237
}
4338
};
4439

45-
const getNodeHeight = <N extends BaseNode>(node: N) => {
46-
const fieldCount = !('fields' in node) || !Array.isArray(node.fields) ? 1 : node.fields.length;
47-
return DEFAULT_NODE_HEADER_HEIGHT + DEFAULT_FIELD_PADDING * 2 + fieldCount * DEFAULT_FIELD_HEIGHT;
48-
};
49-
5040
/**
5141
* Applies a layout to a graph of nodes and edges using the ELK layout engine.
5242
*
@@ -65,12 +55,8 @@ export const applyLayout = <N extends BaseNode, E extends BaseEdge>(
6555
): Promise<ApplyLayout<N, E>> => {
6656
const transformedNodes = nodes.map<N>(node => ({
6757
...node,
68-
height:
69-
'height' in node && typeof node.height === 'number'
70-
? node.height
71-
: (node.measured?.height ?? getNodeHeight(node)),
72-
width:
73-
'width' in node && typeof node.width === 'number' ? node.width : (node.measured?.width ?? DEFAULT_NODE_WIDTH),
58+
height: getNodeHeight(node),
59+
width: getNodeWidth(node),
7460
}));
7561

7662
const transformedEdges = edges.map<ElkExtendedEdge>(edge => ({

‎src/utilities/convert-nodes.test.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
convertToInternalNodes,
88
} from '@/utilities/convert-nodes';
99
import { EMPLOYEES_NODE, ORDERS_NODE } from '@/mocks/datasets/nodes';
10+
import { DEFAULT_FIELD_HEIGHT, DEFAULT_NODE_WIDTH } from '@/utilities/constants';
1011

1112
describe('convert-nodes', () => {
1213
describe('convertToExternalNode', () => {
@@ -144,7 +145,17 @@ describe('convert-nodes', () => {
144145

145146
describe('convertToInternalNodes', () => {
146147
it('Should convert node props to internal node', () => {
147-
const internalNodes = convertToInternalNodes([{ ...ORDERS_NODE, disabled: true }, EMPLOYEES_NODE]);
148+
const internalNodes = convertToInternalNodes([
149+
{
150+
...ORDERS_NODE,
151+
measured: {
152+
width: DEFAULT_NODE_WIDTH,
153+
height: DEFAULT_FIELD_HEIGHT * 2,
154+
},
155+
disabled: true,
156+
},
157+
EMPLOYEES_NODE,
158+
]);
148159
expect(internalNodes).toEqual([
149160
{
150161
id: 'orders',
@@ -174,10 +185,6 @@ describe('convert-nodes', () => {
174185
x: 300,
175186
y: 300,
176187
},
177-
measured: {
178-
height: 72,
179-
width: 244,
180-
},
181188
connectable: false,
182189
data: {
183190
fields: [

‎src/utilities/get-edge-params.test.ts‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,53 @@
11
import { getEdgeParams } from '@/utilities/get-edge-params';
22
import { EMPLOYEES_NODE, ORDERS_NODE } from '@/mocks/datasets/nodes';
3+
import { DEFAULT_FIELD_HEIGHT, DEFAULT_NODE_WIDTH } from '@/utilities/constants';
34

45
describe('get-edge-params', () => {
5-
it('Should get parameters', () => {
6-
const result = getEdgeParams(
7-
{ ...ORDERS_NODE, data: { title: ORDERS_NODE.title, fields: ORDERS_NODE.fields } },
8-
{ ...EMPLOYEES_NODE, data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields } },
9-
);
10-
expect(result).toEqual({
11-
sourcePos: 'bottom',
12-
sx: 240,
13-
sy: 143.5,
14-
targetPos: 'top',
15-
tx: 386,
16-
ty: 292.5,
6+
describe('Without measured heights', () => {
7+
it('Should get parameters', () => {
8+
const result = getEdgeParams(
9+
{ ...ORDERS_NODE, data: { title: ORDERS_NODE.title, fields: ORDERS_NODE.fields } },
10+
{ ...EMPLOYEES_NODE, data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields } },
11+
);
12+
expect(result).toEqual({
13+
sourcePos: 'right',
14+
sx: 269.5,
15+
sy: 180,
16+
targetPos: 'right',
17+
tx: 371.5,
18+
ty: 300,
19+
});
20+
});
21+
});
22+
23+
describe('With measured heights', () => {
24+
it('Should get parameters', () => {
25+
const result = getEdgeParams(
26+
{
27+
...ORDERS_NODE,
28+
data: { title: ORDERS_NODE.title, fields: ORDERS_NODE.fields },
29+
measured: {
30+
width: DEFAULT_NODE_WIDTH,
31+
height: DEFAULT_FIELD_HEIGHT * 2,
32+
},
33+
},
34+
{
35+
...EMPLOYEES_NODE,
36+
data: { title: EMPLOYEES_NODE.title, fields: EMPLOYEES_NODE.fields },
37+
measured: {
38+
width: DEFAULT_NODE_WIDTH,
39+
height: DEFAULT_FIELD_HEIGHT * 4,
40+
},
41+
},
42+
);
43+
expect(result).toEqual({
44+
sourcePos: 'bottom',
45+
sx: 240,
46+
sy: 143.5,
47+
targetPos: 'top',
48+
tx: 386,
49+
ty: 292.5,
50+
});
1751
});
1852
});
1953
});

0 commit comments

Comments
 (0)