Skip to content

Conversation

@csouchet
Copy link
Contributor

@csouchet csouchet commented Mar 8, 2023

Similar to #207

Closes #14

@csouchet csouchet added depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first enhancement New feature or request labels Mar 8, 2023
@github-actions
Copy link

github-actions bot commented Mar 8, 2023

♻️ PR Preview ba84537 has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

@csouchet csouchet marked this pull request as ready for review March 13, 2023 12:22
@csouchet csouchet force-pushed the 14-Allow_to_set_the_Overlay_position branch from 89ccd23 to 81ac928 Compare March 13, 2023 12:23
@csouchet csouchet force-pushed the 14-Allow_to_set_the_Overlay_position branch from 81ac928 to 6b030db Compare March 13, 2023 13:43
@csouchet csouchet force-pushed the 14-Allow_to_set_the_Overlay_position branch from 6b030db to 11411c0 Compare March 13, 2023 13:44
@csouchet csouchet removed the depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first label Mar 13, 2023
@csouchet csouchet requested a review from tbouffard March 13, 2023 13:46
Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

LGTM, I like the refactor of the JS bridge code. Easier to read and more compact.
I suggested a minor changes in the documentation to have a more complete live example.

style: overlay.style ?? (x.enableDefaultOverlayStyle && buildDefaultOverlayStyle(elementsByIds[0].bpmnSemantic.isShape)),
position: elementsByIds[0].bpmnSemantic.isShape ? 'top-center' : 'middle',
};
renderValue: function({bpmnContent, overlays, enableDefaultOverlayStyle}) {
Copy link
Member

Choose a reason for hiding this comment

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

praise: nice to use object destructuring. This makes the code easier to read!

@csouchet csouchet force-pushed the 14-Allow_to_set_the_Overlay_position branch from ddb6eeb to ba84537 Compare March 13, 2023 14:34
@csouchet csouchet merged commit f5bcfbd into main Mar 13, 2023
@csouchet csouchet deleted the 14-Allow_to_set_the_Overlay_position branch March 13, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] Allow to set the Overlay position

3 participants