feat: add support for 'BPMN in Color' #2614
Conversation
Support the 'BPMN in Color' and use bpmn.io colors as fallback. This introduces internal management of BPMN extensions for BPMNShape, BPMNEdge and BPMNLabel. The parsing tests are using diagrams taken from the 'BPMN in Color' specification sample and from the original issue requesting support for modeling colors (#2588) as an example of bpmn.io colors.
tbouffard
added
enhancement
|
♻️ PR Preview 4ee509e has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
|
♻️ PR Preview 4ee509e has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
tbouffard
added
BPMN rendering
tbouffard
added
BPMN support
…fault in StyleComputer
# Conflicts: # src/component/parser/json/converter/DiagramConverter.ts # test/unit/component/mxgraph/renderer/StyleComputer.test.ts # test/unit/component/parser/BpmnParser.test.ts # test/unit/helpers/JsonTestUtils.ts # test/unit/helpers/bpmn-model-expect.ts
…c method to handle extensions)
This proves the need for optional chaining for font management
tbouffard
removed
the
WIP 🚧
# Conflicts: # src/component/BpmnVisualization.ts # src/component/options.ts # test/unit/component/mxgraph/renderer/StyleComputer.test.ts
| } | ||
|
|
||
| // 'BPMN in Color' extensions with fallback to bpmn.io colors | ||
| private static setColorExtensionsOnShape(shape: Shape, bpmnShape: BPMNShape): void { |
There was a problem hiding this comment.
suggestion:
I think we don't need to check if background-color, fill, border-color and stroke are in bpmnShape, because even if there are undefined, with this implementation or the suggested implementation, the attributes of shape.extensions will be undefined.
| private static setColorExtensionsOnShape(shape: Shape, bpmnShape: BPMNShape): void { | |
| private static setColorExtensionsOnShape(shape: Shape, bpmnShape: BPMNShape): void { | |
| shape.extensions.fillColor = <string>bpmnShape['background-color'] ?? <string>bpmnShape['fill']; | |
| shape.extensions.strokeColor = <string>bpmnShape['border-color'] ?? <string>bpmnShape['stroke']; | |
| } |
There was a problem hiding this comment.
The usage of the color extensions is very rare, so the idea was to not have properties with undefined values but to have no property in this case.
The model is not stored internally, so I will give it a try to your proposal to simplify the code here and for Edge.
There was a problem hiding this comment.
@csouchet the proposal cannot be implemented out of the box
const backgroundColor = bpmnShape['background-color'];generates a TS error TS7053: Element implicitly has an 'any' type because expression of type '"background-color"' can't be used to index type 'BPMNShape'. Property 'background-color' does not exist on type 'BPMNShape'.
I can ignore the error but the resulting code is less lisible and ignoring error is misleading
// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- explain why!
// @ts-ignore
shape.extensions.fillColor = <string>bpmnShape['background-color'] ?? <string>bpmnShape['fill'];
// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- explain why!
// @ts-ignore
shape.extensions.strokeColor = <string>bpmnShape['border-color'] ?? <string>bpmnShape['stroke'];|
|
||
| return new Edge(edge.id, flow, waypoints, label, messageVisibleKind); | ||
| const edge = new Edge(bpmnEdge.id, flow, waypoints, label, messageVisibleKind); | ||
| DiagramConverter.setColorExtensionsOnEdge(edge, bpmnEdge); |
| }) | ||
| .filter(Boolean); | ||
| } | ||
|
|
||
| // 'BPMN in Color' extensions with fallback to bpmn.io colors | ||
| private static setColorExtensionsOnEdge(edge: Edge, bpmnEdge: BPMNEdge): void { | ||
| if ('border-color' in bpmnEdge) { |
There was a problem hiding this comment.
suggestion:
| if ('border-color' in bpmnEdge) { | |
| edge.extensions.strokeColor = <string>bpmnEdge['border-color'] ?? <string>bpmnEdge['stroke']; | |
| } |
|
|
||
| const label = new Label(font, bounds); | ||
| if ('color' in bpmnLabel) { | ||
| label.extensions.color = <string>bpmnLabel.color; |
There was a problem hiding this comment.
question:
Same as for Shape & Edge
There was a problem hiding this comment.
Here this would change the behavior.
We return a Label instance only if properties are set (previously only font and bounds.
If I change the implementation as for shape and edge, we will always have an instance even when the label color is undefined.
So, I prefer not to change it.
|
Kudos, SonarCloud Quality Gate passed! |
Support the 'BPMN in Color' and use bpmn.io colors as fallback.
The BPMN parser always extract the colors, but by default, they are ignored by the Renderer. It is possible to enable the support in the Renderer by setting an options at the library initialization.
This introduces internal management of BPMN extensions for BPMNShape, BPMNEdge and BPMNLabel.
The parsing tests are using diagrams taken from the 'BPMN in Color' specification sample (this ensure that the implementation is compliant with the specification) and from the original issue requesting support for modeling colors as an example of bpmn.io colors.
closes #2588
Tasks
DiagramConverter.deserializeShape: "Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed." (feedback from SonarCloud)ignoreBpmnColorsoption to enable/disable the rendering supportDecision about support by default of "BPMN in Color"
Decision with @csouchet on
2023-04-12ignoreBpmnColorswhich istrueby default. The new option involves the "BPMN renderer", so it must be consistent with the new options we plan to introduce in the future. See [FEAT] Define the options that will be used to configure the Renderer #2659.Screenshots
sample from the "BPMN in Color" Specification
Available in https://github.com/bpmn-miwg/bpmn-in-color, also stored for use in XML parsing unit tests
adapted from the diagram provided in #2588
Also used in XML parsing unit tests
e2e test diagram, including message flow icons that are also colored
Files from miwg-test-suite