-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: simplify shapes registration #3045
Conversation
The private method that registered the shapes was a pure function, so it had been extracted from the ShapeConfigurator class. The registration code has also been simplified to better reflect a configuration pattern (less explicit calls to the mxGraph registration code).
♻️ PR Preview 997ebf6 has been successfully destroyed since this PR has been closed. 🤖 By surge-preview |
♻️ PR Preview 997ebf6 has been successfully destroyed since this PR has been closed. 🤖 By surge-preview |
@@ -36,47 +38,53 @@ import { ComplexGatewayShape, EventBasedGatewayShape, ExclusiveGatewayShape, Inc | |||
import { TextAnnotationShape } from '../shape/text-annotation-shapes'; | |||
import { BpmnStyleIdentifier } from '../style'; | |||
|
|||
const registerShapes = (): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: the shapes are registered in a global state in mxGraph, so bpmn-visualization
currently registers the same shapes at every instantiation of the BpmnVisualization
class.
We could register the shapes only once.
This is what is done in maxGraph: https://github.com/maxGraph/maxGraph/blob/v0.8.0/packages/core/src/view/cell/register-shapes.ts#L36-L43
Quality Gate passedIssues Measures |
The private method that registered the shapes was a pure function, so it had been extracted from the ShapeConfigurator class.
The registration code has also been simplified to better reflect a configuration pattern (less explicit calls to the mxGraph registration code).
Notes
Part of initiative including #3041, #3042 and #3043 (more to come).