refactor: inject IconPainter in Shapes#3042
Merged
Merged
Conversation
Previously, the shapes relied on `IconPainterProvider` to get the `IconPainter` instance. This instance is now injected at shape instantiation by `BpmnCellRenderer`. This reduces the adherence to a global state. This is a first step to remove the `IconPainterProvider` global state. This may be done later as it would imply to: - introduce a new configuration options, for example a `render.iconPainter` property - change how the `BpmnGraph` is initialized and configured - manage the breaking changes introduced by the provider removal
|
♻️ PR Preview d9055bd has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
|
♻️ PR Preview d9055bd has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
tbouffard
changed the title
IconPainter in Shapes
csouchet
approved these changes
Mar 1, 2024
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the shapes relied on
IconPainterProviderto get theIconPainterinstance. This instance is now injected at shape instantiation byBpmnCellRenderer. This reduces the adherence to a global state.This is a first step to remove the
IconPainterProviderglobal state. This may be done later as it would imply to:render.iconPainterpropertyBpmnGraphis initialized and configuredMinor functional change
Previously, when a shape was rendered, the current instance of the
IconPainterwas injected. If, between two renders, a new instance ofIconPainterwas configured viaIconPainterProvider, it was used in the next render. This was a side-effect, not a desired feature, and was not documented.The interest is limited: we haven't yet seen a use case that requires it. The existing example of modifying the user task icon doesn't need it: in this case, we still want to display the customized icon.
With the changes proposed here, the behavior is modified. The
IconPainterinstance is defined when the library is initialized, so its modification inIconPainterProvideronly applies to new instances of the library. All rendering of a shape will always use theIconPainterinstance defined when the library was initialized.