feat: allow to style BPMN Elements #247

csouchet · 2023-09-12T14:49:50Z

This pull request includes changes to several files in the inst/htmlwidgets and R directories. Here is a description of the changes made in some files:

R/funs.R:
- Added a new function create_element_style which creates the correct style structure for BPMN elements.
- Added a new function create_edge_style which creates the correct style structure for BPMN edges.
- Added a new function create_shape_style to create the correct style structure for BPMN shapes.
- Added a new `create_gradient_fill' function to create a gradient fill style for BPMN shapes.
- Modified the create_stroke, create_font and create_fill functions to accept additional style properties.
- Updated the build_bpmnContent function to accept a bpmnElementStyles parameter, which is a list of existing elements with their style to apply.
R/bpmnVisualizationR.R:
- Updated the display function to accept a bpmnElementStyles parameter which allows to specify styles for BPMN elements.
- An example of how to use bpmnElementStyles to define styles for BPMN elements has been added to the function documentation.

To build the documentation

Closes #13

github-actions · 2023-09-12T14:53:01Z

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

_{🤖 By surge-preview}

* checking PDF version of manual ... WARNING Warning: LaTeX errors when creating PDF version. This typically indicates Rd problems. LaTeX errors found: ! LaTeX Error: Unicode character ⚠ (U+26A0) not set up for use with LaTeX.

tbouffard

✔️ Implementation
✔️ Great documentation (including documentation section reuse)
✔️ A lot of new tests
❌ README

missing API usage in the README of the repository to be consistent with all existing features that are referenced there
Consider updating the illustration image: use script of [DOC] Update the hero image #72 and [DOC] Add hero image in the README #45 (we should put the script in the doc folder instead of keeping it in comments of Pull Requests and probably rename this folder into docs for consistency with other repositories)

⚠️ Side note about the PR description: IHMO, there is no need for such a PR description when I am a reviewer. I think it was written by a ChatBot as it is very wordy and doesn't provide the information I expect to find to guide my review and write the merge commit.

It is mainly a list of added/updated functions and the code and R documentation should be enough to provide the same information. If not, this means that the code and/or the documentation are uncleared
it only states what is done where I expect to find explanations about why it is done. The code should be clear enough about the what (and if required, add code comments). I would prefer to have a clear text about the "doc section" reuse that is currently hidden in the list of links of the "to build the documentation" paragraph.

I've recently realized that this is becoming the norm in the latest PRs you've opened, and I suggest that it be adapted to help my review. Let's discuss it in person to find a solution that works for both of us.

R/funs.R

inst/htmlwidgets/bpmnVisualizationR.js

tbouffard · 2023-09-13T07:29:57Z

R/funs.R

+#'  
+#' @keywords internal
+create_element_style <- function(elementIds,
+                                 stroke_color = NULL,


thought: I suppose it's the R way to have dozens of parameters in a function, but it's boring and a pain to read.

tbouffard

LGTM
ℹ️ the image in the Readme could still be updated

tbouffard · 2023-09-15T13:41:31Z

doc/example_email_voting_with_overlays.svg

WARN the label of the activity on top of the subprocess is not centered
The R script used to produced this image is not shared

I didn't use any script. I just edited the SVG image with Inkscape.

ℹ️ When I created this image for the first time, it was from a HTML page and it take me a long time to clean the SVG and to see the label.

I didn't understand which activity that you mention. The label is centered:

Here is how it is rendered with Firefox 117 on Ubuntu 20.04. I don't see the problem with Chrome 117.
See also the sendTask in the left

There was no issue with the previous image on Firefox.
We could create a dedicated issue to handle it.

ℹ️ When I created this image for the first time, it was from a HTML page and it take me a long time to clean the SVG and to see the label.

However, we need the script that is able to produce this rendering to be able to reproduce it.
I understand the pain about the label repositioning. Be aware of https://github.com/process-analytics/bpmn-visualization-tools/tree/v0.4.0/bpmn-visualization-svg-screenshots for the next time

As per decision with @csouchet this will be handle with #251 and #252

csouchet added 3 commits September 12, 2023 16:45

add fun functions to create style for edges and shapes

14fcec0

add bpmnElementStyles parameter to display function

1e1e213

genarate doc

344aa59

csouchet added the enhancement New feature or request label Sep 12, 2023

csouchet added 3 commits September 12, 2023 16:59

Update doc: create_overlay_style

43bf0f8

Fix:

1b0ece4

* checking PDF version of manual ... WARNING Warning: LaTeX errors when creating PDF version. This typically indicates Rd problems. LaTeX errors found: ! LaTeX Error: Unicode character ⚠ (U+26A0) not set up for use with LaTeX.

update comment doc

be77a8d

csouchet marked this pull request as ready for review September 12, 2023 15:36

csouchet requested a review from tbouffard September 12, 2023 15:36

tbouffard requested changes Sep 13, 2023

View reviewed changes

csouchet and others added 2 commits September 13, 2023 14:35

Update the readme

45ab163

Update R/funs.R

e2e6ae4

tbouffard approved these changes Sep 13, 2023

View reviewed changes

csouchet and others added 2 commits September 15, 2023 10:12

Merge branch 'main' into 13-Allow_to_style_BPMN_Elements

6b094b0

update svg

3cffa24

tbouffard reviewed Sep 15, 2023

View reviewed changes

This was referenced Sep 18, 2023

[DOC] Document how to create the image of the README #251

Open

[DOC] Labels wrongly positionned in the image of the README #252

Open

csouchet merged commit c28f64b into main Sep 18, 2023
4 checks passed

csouchet deleted the 13-Allow_to_style_BPMN_Elements branch September 18, 2023 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow to style BPMN Elements #247

feat: allow to style BPMN Elements #247

csouchet commented Sep 12, 2023 •

edited

Loading

github-actions bot commented Sep 12, 2023 •

edited

Loading

tbouffard left a comment

tbouffard Sep 13, 2023

tbouffard left a comment

tbouffard Sep 15, 2023

csouchet Sep 15, 2023 •

edited

Loading

csouchet Sep 15, 2023

tbouffard Sep 15, 2023 •

edited

Loading

tbouffard Sep 18, 2023

feat: allow to style BPMN Elements #247

feat: allow to style BPMN Elements #247

Conversation

csouchet commented Sep 12, 2023 • edited Loading

To build the documentation

github-actions bot commented Sep 12, 2023 • edited Loading

tbouffard left a comment

Choose a reason for hiding this comment

tbouffard Sep 13, 2023

Choose a reason for hiding this comment

tbouffard left a comment

Choose a reason for hiding this comment

tbouffard Sep 15, 2023

Choose a reason for hiding this comment

csouchet Sep 15, 2023 • edited Loading

Choose a reason for hiding this comment

csouchet Sep 15, 2023

Choose a reason for hiding this comment

tbouffard Sep 15, 2023 • edited Loading

Choose a reason for hiding this comment

tbouffard Sep 18, 2023

Choose a reason for hiding this comment

csouchet commented Sep 12, 2023 •

edited

Loading

github-actions bot commented Sep 12, 2023 •

edited

Loading

csouchet Sep 15, 2023 •

edited

Loading

tbouffard Sep 15, 2023 •

edited

Loading