Skip to content

Commit 17c2a29

Browse files
lordriplordrip
committed
feat(Canvas): Filter placeholder nodes in screenshots and docs
Currently, there are multiple placeholder nodes across the Canvas making interacting with the flows easier. The downside is that when exporting screenshots or documentation about the flow, all these placeholders are included, making the picture more complicated than it needs to be. The fix is to filter all placeholders before drawing the graph. In addition to that, both `ExportDocumentPreviewModal` and `FlowExportImage` components should use the same `<HiddenCanvas>` component to have a consistent image. A follow-up is needed to consolidate all `placeholder-related` logic, because in some cases, the `placeholder` wording gets added to the path, while in others not. fix: #3029
1 parent 89dd298 commit 17c2a29

14 files changed

Lines changed: 646 additions & 117 deletions

File tree

‎packages/ui-tests/cypress/support/next-commands/nodeConfiguration.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ Cypress.Commands.add('generateDocumentationPreview', () => {
159159
});
160160

161161
Cypress.Commands.add('documentationTableCompare', (routeName: string, expectedTableData: string[][]) => {
162+
// Wait until the loading distractor is no longer in the document
163+
cy.get('[data-testid="Loading markdown preview"]').should('not.exist', { timeout: 30_000 });
164+
162165
cy.contains('h1', routeName)
163166
.next('table')
164167
.find('.pf-v6-c-table__tbody')

‎packages/ui/src/components/Visualization/Canvas/__snapshots__/flow.service.test.ts.snap‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,189 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`FlowService getFlowDiagram should remove placeholders 1`] = `
4+
[
5+
{
6+
"children": [],
7+
"data": {},
8+
"group": false,
9+
"height": 75,
10+
"id": "test|route.from",
11+
"parentNode": "test|route",
12+
"shape": "rect",
13+
"type": "node",
14+
"width": 90,
15+
},
16+
{
17+
"children": [],
18+
"data": {},
19+
"group": false,
20+
"height": 75,
21+
"id": "test|route.from.steps.0.set-header",
22+
"parentNode": "test|route",
23+
"shape": "rect",
24+
"type": "node",
25+
"width": 90,
26+
},
27+
{
28+
"children": [],
29+
"data": {},
30+
"group": false,
31+
"height": 75,
32+
"id": "test|route.from.steps.1.choice.when.0.steps.0.log",
33+
"parentNode": "test|route.from.steps.1.choice.when.0",
34+
"shape": "rect",
35+
"type": "node",
36+
"width": 90,
37+
},
38+
{
39+
"children": [
40+
"test|route.from.steps.1.choice.when.0.steps.0.log",
41+
],
42+
"data": {},
43+
"group": true,
44+
"id": "test|route.from.steps.1.choice.when.0",
45+
"label": "route.from.steps.1.choice.when.0",
46+
"parentNode": "test|route.from.steps.1.choice",
47+
"style": {
48+
"padding": 40,
49+
},
50+
"type": "group",
51+
},
52+
{
53+
"children": [],
54+
"data": {},
55+
"group": false,
56+
"height": 75,
57+
"id": "test|route.from.steps.1.choice.otherwise.steps.0.to",
58+
"parentNode": "test|route.from.steps.1.choice.otherwise",
59+
"shape": "rect",
60+
"type": "node",
61+
"width": 90,
62+
},
63+
{
64+
"children": [],
65+
"data": {},
66+
"group": false,
67+
"height": 75,
68+
"id": "test|route.from.steps.1.choice.otherwise.steps.1.to",
69+
"parentNode": "test|route.from.steps.1.choice.otherwise",
70+
"shape": "rect",
71+
"type": "node",
72+
"width": 90,
73+
},
74+
{
75+
"children": [],
76+
"data": {},
77+
"group": false,
78+
"height": 75,
79+
"id": "test|route.from.steps.1.choice.otherwise.steps.2.log",
80+
"parentNode": "test|route.from.steps.1.choice.otherwise",
81+
"shape": "rect",
82+
"type": "node",
83+
"width": 90,
84+
},
85+
{
86+
"children": [
87+
"test|route.from.steps.1.choice.otherwise.steps.0.to",
88+
"test|route.from.steps.1.choice.otherwise.steps.1.to",
89+
"test|route.from.steps.1.choice.otherwise.steps.2.log",
90+
],
91+
"data": {},
92+
"group": true,
93+
"id": "test|route.from.steps.1.choice.otherwise",
94+
"label": "route.from.steps.1.choice.otherwise",
95+
"parentNode": "test|route.from.steps.1.choice",
96+
"style": {
97+
"padding": 40,
98+
},
99+
"type": "group",
100+
},
101+
{
102+
"children": [
103+
"test|route.from.steps.1.choice.when.0",
104+
"test|route.from.steps.1.choice.otherwise",
105+
],
106+
"data": {},
107+
"group": true,
108+
"id": "test|route.from.steps.1.choice",
109+
"label": "route.from.steps.1.choice",
110+
"parentNode": "test|route",
111+
"style": {
112+
"padding": 40,
113+
},
114+
"type": "group",
115+
},
116+
{
117+
"children": [],
118+
"data": {},
119+
"group": false,
120+
"height": 75,
121+
"id": "test|route.from.steps.2.to",
122+
"parentNode": "test|route",
123+
"shape": "rect",
124+
"type": "node",
125+
"width": 90,
126+
},
127+
{
128+
"children": [
129+
"test|route.from",
130+
"test|route.from.steps.0.set-header",
131+
"test|route.from.steps.1.choice",
132+
"test|route.from.steps.2.to",
133+
],
134+
"data": {},
135+
"group": true,
136+
"id": "test|route",
137+
"label": "route",
138+
"parentNode": undefined,
139+
"style": {
140+
"padding": 40,
141+
},
142+
"type": "group",
143+
},
144+
]
145+
`;
146+
147+
exports[`FlowService getFlowDiagram should remove placeholders 2`] = `
148+
[
149+
{
150+
"edgeStyle": "solid",
151+
"id": "test|route.from >>> route.from.steps.0.set-header",
152+
"source": "test|route.from",
153+
"target": "test|route.from.steps.0.set-header",
154+
"type": "edge",
155+
},
156+
{
157+
"edgeStyle": "solid",
158+
"id": "test|route.from.steps.0.set-header >>> route.from.steps.1.choice",
159+
"source": "test|route.from.steps.0.set-header",
160+
"target": "test|route.from.steps.1.choice",
161+
"type": "edge",
162+
},
163+
{
164+
"edgeStyle": "solid",
165+
"id": "test|route.from.steps.1.choice.otherwise.steps.0.to >>> route.from.steps.1.choice.otherwise.steps.1.to",
166+
"source": "test|route.from.steps.1.choice.otherwise.steps.0.to",
167+
"target": "test|route.from.steps.1.choice.otherwise.steps.1.to",
168+
"type": "edge",
169+
},
170+
{
171+
"edgeStyle": "solid",
172+
"id": "test|route.from.steps.1.choice.otherwise.steps.1.to >>> route.from.steps.1.choice.otherwise.steps.2.log",
173+
"source": "test|route.from.steps.1.choice.otherwise.steps.1.to",
174+
"target": "test|route.from.steps.1.choice.otherwise.steps.2.log",
175+
"type": "edge",
176+
},
177+
{
178+
"edgeStyle": "solid",
179+
"id": "test|route.from.steps.1.choice >>> route.from.steps.2.to",
180+
"source": "test|route.from.steps.1.choice",
181+
"target": "test|route.from.steps.2.to",
182+
"type": "edge",
183+
},
184+
]
185+
`;
186+
3187
exports[`FlowService getFlowDiagram should return nodes and edges for a group with children 1`] = `
4188
[
5189
{

‎packages/ui/src/components/Visualization/Canvas/flow.service.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { CatalogKind } from '../../../models';
22
import { EntityType } from '../../../models/camel/entities';
33
import { CamelRouteVisualEntity, createVisualizationNode } from '../../../models/visualization';
4+
import { camelRouteJson } from '../../../stubs/camel-route';
45
import { FlowService } from './flow.service';
56

67
describe('FlowService', () => {
@@ -127,6 +128,18 @@ describe('FlowService', () => {
127128
expect(group.group).toBeTruthy();
128129
});
129130

131+
it('should remove placeholders', () => {
132+
const routeNode = new CamelRouteVisualEntity(camelRouteJson).toVizNode();
133+
134+
const { nodes, edges } = FlowService.getFlowDiagram('test', routeNode, { removePlaceholder: true });
135+
nodes.forEach((node) => {
136+
delete node.data!.vizNode;
137+
});
138+
139+
expect(nodes).toMatchSnapshot();
140+
expect(edges).toMatchSnapshot();
141+
});
142+
130143
it('should scope nodes & edges IDs', () => {
131144
const routeNode = new CamelRouteVisualEntity({
132145
route: { id: 'route-8888', from: { uri: 'timer:clock', steps: [{ to: { uri: 'log' } }] } },

‎packages/ui/src/components/Visualization/Canvas/flow.service.ts‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@ export class FlowService {
99
static edges: CanvasEdge[] = [];
1010
private static visitedNodes: string[] = [];
1111

12-
static getFlowDiagram(scope: string, vizNode: IVisualizationNode): CanvasNodesAndEdges {
12+
static getFlowDiagram(
13+
scope: string,
14+
vizNode: IVisualizationNode,
15+
options: { removePlaceholder?: boolean } = {},
16+
): CanvasNodesAndEdges {
1317
this.nodes = [];
1418
this.edges = [];
1519
this.visitedNodes = [];
1620

17-
this.appendNodesAndEdges(vizNode);
21+
this.appendNodesAndEdges(vizNode, options);
1822

1923
this.nodes.forEach((node) => {
2024
node.id = `${scope}|${node.id}`;
@@ -31,19 +35,25 @@ export class FlowService {
3135
}
3236

3337
/** Method for iterating over all the IVisualizationNode and its children using a depth-first algorithm */
34-
private static appendNodesAndEdges(vizNodeParam: IVisualizationNode): void {
35-
if (this.visitedNodes.includes(vizNodeParam.id)) {
38+
private static appendNodesAndEdges(
39+
vizNodeParam: IVisualizationNode,
40+
options: { removePlaceholder?: boolean } = {},
41+
): void {
42+
const removePlaceholder = options.removePlaceholder ?? false;
43+
if (this.visitedNodes.includes(vizNodeParam.id) || (removePlaceholder && vizNodeParam.data.isPlaceholder)) {
3644
return;
3745
}
38-
3946
let node: CanvasNode;
4047

41-
const children = vizNodeParam.getChildren() ?? [];
48+
let children = vizNodeParam.getChildren() ?? [];
49+
if (removePlaceholder) {
50+
children = children.filter((child) => !child.data.isPlaceholder);
51+
}
4252
const hasRealChildren = children.length > 0;
4353

4454
if (vizNodeParam.data.isGroup && hasRealChildren) {
4555
children.forEach((child) => {
46-
this.appendNodesAndEdges(child);
56+
this.appendNodesAndEdges(child, options);
4757
});
4858

4959
node = this.getGroup(vizNodeParam.id, {
@@ -64,7 +74,7 @@ export class FlowService {
6474
this.visitedNodes.push(node.id);
6575

6676
/** Add edges */
67-
this.edges.push(...this.getEdgesFromVizNode(vizNodeParam));
77+
this.edges.push(...this.getEdgesFromVizNode(vizNodeParam, options));
6878
}
6979

7080
private static getCanvasNode(vizNodeParam: IVisualizationNode): CanvasNode {
@@ -86,11 +96,19 @@ export class FlowService {
8696
return canvasNode;
8797
}
8898

89-
private static getEdgesFromVizNode(vizNodeParam: IVisualizationNode): CanvasEdge[] {
99+
private static getEdgesFromVizNode(
100+
vizNodeParam: IVisualizationNode,
101+
options: { removePlaceholder?: boolean } = {},
102+
): CanvasEdge[] {
90103
const edges: CanvasEdge[] = [];
91104
const prev = vizNodeParam.getPreviousNode?.();
92105
const next = vizNodeParam.getNextNode?.();
93106

107+
const removePlaceholder = options.removePlaceholder ?? false;
108+
if (removePlaceholder && next?.data.isPlaceholder) {
109+
return edges;
110+
}
111+
94112
const isGroup = vizNodeParam.data?.isGroup === true;
95113
const hasChildren = (vizNodeParam.getChildren() ?? []).length > 0;
96114

‎packages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocument.test.tsx‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
import { act, fireEvent, render } from '@testing-library/react';
1+
import { VisualizationProvider } from '@patternfly/react-topology';
2+
import { act, fireEvent, render, waitFor } from '@testing-library/react';
23

34
import { CamelResource, CamelRouteResource } from '../../../../models/camel';
45
import { EntityType } from '../../../../models/camel/entities';
56
import { TestProvidersWrapper } from '../../../../stubs';
7+
import { ControllerService } from '../../Canvas/controller.service';
68
import { ExportDocument } from './ExportDocument';
79

810
describe('FlowExportDocument.tsx', () => {
@@ -19,9 +21,11 @@ describe('FlowExportDocument.tsx', () => {
1921
jest.spyOn(console, 'error').mockImplementation(() => {});
2022
const { Provider } = TestProvidersWrapper({ camelResource });
2123
const wrapper = render(
22-
<Provider>
23-
<ExportDocument />
24-
</Provider>,
24+
<VisualizationProvider controller={ControllerService.createController()}>
25+
<Provider>
26+
<ExportDocument />
27+
</Provider>
28+
</VisualizationProvider>,
2529
);
2630

2731
const exportButton = wrapper.getByTestId('documentationPreviewButton');
@@ -42,9 +46,14 @@ describe('FlowExportDocument.tsx', () => {
4246
fireEvent.click(showAllBtn);
4347
});
4448

45-
const headers = await wrapper.findAllByTestId('export-document-preview-h1');
46-
expect(headers.length).toEqual(3);
47-
const tables = await wrapper.findAllByTestId('export-document-preview-table');
48-
expect(tables.length).toEqual(1);
49+
await waitFor(async () => {
50+
const header = await wrapper.findByTestId('documentationPreviewModal');
51+
expect(header).toBeInTheDocument();
52+
});
53+
54+
await waitFor(async () => {
55+
const tables = await wrapper.findAllByTestId('export-document-preview-body');
56+
expect(tables.length).toEqual(1);
57+
});
4958
});
5059
});

‎packages/ui/src/components/Visualization/ContextToolbar/ExportDocument/ExportDocument.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@ export const ExportDocument: FunctionComponent = () => {
2222
variant="control"
2323
data-testid="documentationPreviewButton"
2424
/>
25-
{isModalOpen && <ExportDocumentPreviewModal onClose={() => setIsModalOpen(false)} />}
25+
{isModalOpen && (
26+
<ExportDocumentPreviewModal
27+
onClose={() => {
28+
setIsModalOpen(false);
29+
}}
30+
/>
31+
)}
2632
</>
2733
);
2834
};

0 commit comments

Comments
 (0)
⚡