Skip to content

Commit 337b45a

Browse files
authored
Refine spacing for nested parallel with sibling that has branch label (#1316)
1 parent f572523 commit 337b45a

3 files changed

Lines changed: 22 additions & 9 deletions

File tree

src/main/frontend/pipeline-graph-view/pipeline-graph/main/NestedPipelineGraphLayout.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ function buildGraphNested(
218218
};
219219
hasParallel = false;
220220
}
221-
const isChainedParallel =
221+
const isNestedParallel =
222222
isParallel &&
223223
stage.children.length > 0 &&
224224
stage.children[0].children.length > 0 &&
@@ -232,8 +232,8 @@ function buildGraphNested(
232232
isParallel &&
233233
hasChildren &&
234234
// Do not add a branch label on the parent of a nested parallel. Instead, show a big label on the nested parallel block.
235-
!isChainedParallel;
236-
const isHidden = hasBranchLabel || hasChildren || isChainedParallel;
235+
!isNestedParallel;
236+
const isHidden = hasBranchLabel || hasChildren || isNestedParallel;
237237
const hasBigLabel =
238238
hasParallel ||
239239
(stage.type === "STAGE" && hasChildren && !hasBranchLabel) ||
@@ -242,6 +242,7 @@ function buildGraphNested(
242242
const hasSmallLabel = !isHidden && !hasBigLabel;
243243
const childNode: GraphNode = {
244244
...makeNodeForStage(stage, layout),
245+
isNestedParallel,
245246
isParallel,
246247
isSkipped,
247248
isHidden,
@@ -253,19 +254,24 @@ function buildGraphNested(
253254
};
254255
buildGraphNested(childNode, stage.children, layout, isLast);
255256
if (hasBigLabel) childNode.shiftY += layout.labelOffsetV;
257+
childNode.allChildrenSkipped =
258+
childNode.hasParallel && !childNode.children.some((c) => !c.isSkipped);
259+
node.children.push(childNode);
260+
}
261+
// Two phased approach as we need to detect if any sibling has a branch label -> node will need to make space.
262+
const anyChildHasBranchLabel =
263+
node.hasParallel && node.children.some((c) => c.hasBranchLabel);
264+
for (const childNode of node.children) {
256265
if (
257-
isChainedParallel ||
266+
(childNode.isNestedParallel && !anyChildHasBranchLabel) ||
258267
(childNode.hasParallel &&
259268
childNode.children.some((c) => c.hasBranchLabel))
260269
) {
261-
// - Nested parallel children, avoid collapsing curves.
262-
// - Any child has branch label, make space for branch label.
270+
// - Nested parallel children (unless a sibling already added spacing on the parent), avoid collapsing curves.
271+
// - Any of its children has branch label, make space for branch labels.
263272
childNode.shiftX += layout.nodeSpacingH;
264273
childNode.width += layout.nodeSpacingH;
265274
}
266-
childNode.allChildrenSkipped =
267-
childNode.hasParallel && !childNode.children.some((c) => !c.isSkipped);
268-
node.children.push(childNode);
269275
}
270276
if (node.hasParallel) {
271277
// Move shiftY from first parallel child up one level.
@@ -603,6 +609,7 @@ export function removeFalseOptionalGraphNodeFlags(node: GraphNode) {
603609
if (!node.hasStageEnd) delete node.hasStageEnd;
604610
if (!node.hasTiming) delete node.hasTiming;
605611
if (!node.isHidden) delete node.isHidden;
612+
if (!node.isNestedParallel) delete node.isNestedParallel;
606613
if (!node.isParallel) delete node.isParallel;
607614
if (!node.isSkipped) delete node.isSkipped;
608615

src/main/frontend/pipeline-graph-view/pipeline-graph/main/PipelineGraphModel.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ export type GraphNode = {
109109
height: number;
110110
allChildrenSkipped?: boolean;
111111
isHidden?: boolean;
112+
isNestedParallel?: boolean;
112113
isParallel?: boolean;
113114
isSkipped?: boolean;
114115
firstChildIsSkipped?: boolean;

src/main/frontend/pipeline-graph-view/pipeline-graph/main/__snapshots__/NestedPipelineGraphLayout.spec.ts.snap

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10239,6 +10239,7 @@ exports[`NestedPipelineGraphLayout > nestedGraphLayout > gh1155_nestedParallel.j
1023910239
"height": 140,
1024010240
"id": -16,
1024110241
"isHidden": true,
10242+
"isNestedParallel": true,
1024210243
"isParallel": true,
1024310244
"isPlaceholder": false,
1024410245
"key": "n_-16",
@@ -10731,6 +10732,7 @@ exports[`NestedPipelineGraphLayout > nestedGraphLayout > gh1155_nestedParallel.j
1073110732
"height": 140,
1073210733
"id": -16,
1073310734
"isHidden": true,
10735+
"isNestedParallel": true,
1073410736
"isParallel": true,
1073510737
"isPlaceholder": false,
1073610738
"key": "n_-16",
@@ -11177,6 +11179,7 @@ exports[`NestedPipelineGraphLayout > nestedGraphLayout > gh1155_nestedParallel.j
1117711179
"height": 140,
1117811180
"id": -16,
1117911181
"isHidden": true,
11182+
"isNestedParallel": true,
1118011183
"isParallel": true,
1118111184
"isPlaceholder": false,
1118211185
"key": "n_-16",
@@ -11375,6 +11378,7 @@ exports[`NestedPipelineGraphLayout > nestedGraphLayout > gh1155_nestedParallel.j
1137511378
"height": 140,
1137611379
"id": -16,
1137711380
"isHidden": true,
11381+
"isNestedParallel": true,
1137811382
"isParallel": true,
1137911383
"isPlaceholder": false,
1138011384
"key": "n_-16",
@@ -11532,6 +11536,7 @@ exports[`NestedPipelineGraphLayout > nestedGraphLayout > gh1155_nestedParallel.j
1153211536
"height": 140,
1153311537
"id": -16,
1153411538
"isHidden": true,
11539+
"isNestedParallel": true,
1153511540
"isParallel": true,
1153611541
"isPlaceholder": false,
1153711542
"key": "n_-16",

0 commit comments

Comments
 (0)