Skip to content

Conversation

@tbouffard
Copy link
Member

The position (x and y coordinates) were not ignored.
Only the global positioning of the label, relative to the shape was updated.

Also rename some visual tests prior adding new ones, including test for ignore label styles as well.

The position (x and y coordinates) were not ignored.
Only the global positioning of the label, relative to the shape was updated.

Also rename some visual tests prior adding new ones.
@tbouffard tbouffard added the bug Something isn't working label Dec 4, 2025
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

🎊 PR Preview b6f4d82 has been successfully built and deployed to https://process-analytics-bpmn-visualization-js-demo_preview-pr-3434.surge.sh

🕐 Build time: 0.011s

🤖 By surge-preview

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

🎊 PR Preview b6f4d82 has been successfully built and deployed to https://process-analytics-bpmn-visualization-js-doc_preview-pr-3434.surge.sh

🕐 Build time: 0.011s

🤖 By surge-preview

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 4, 2025

export function isLabelBoundsIgnored(shape: Shape, ignoreBpmnActivityLabelBounds: boolean, ignoreBpmnTaskLabelBounds: boolean): boolean {
const kind = shape.bpmnElement.kind;
if (ShapeUtil.isPoolOrLane(kind)) {
return true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: restore comment

Suggested change
return true;
// pool/lane label bounds are not managed for now (use hard coded values)
return true;

Comment on lines +103 to +105
const imageSnapshotConfigurator = ignoreBpmnActivityLabelBounds
? new ImageSnapshotConfigurator(new ImageSnapshotThresholdsActivityLabelBoundsIgnored(), 'bpmn-rendering-ignore-options/ignored')
: new ImageSnapshotConfigurator(new ImageSnapshotThresholdsActivityLabelBounds(), 'bpmn-rendering-ignore-options/not-ignored');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: Use a single MultiBrowserImageSnapshotThresholds class to managed both ignored/not-ignored use case as done in https://github.com/process-analytics/bpmn-visualization-js/blob/v0.47.0/test/e2e/bpmn.theme.test.ts#L68
The key would be the use case and not the name of the diagram

The same applies to the other test

[ShapeBpmnElementKind.GATEWAY_PARALLEL, false],
];

test.each(cases)('should ignore %s label bounds? %s', (kind, expected) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: try to inline the cases variable (apply to all tests)

@tbouffard tbouffard marked this pull request as draft December 5, 2025 09:17
return true;
}

if (ignoreBpmnActivityLabelBounds && ShapeUtil.isActivity(kind)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep a single return including the all conditions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants