Skip to content

Conversation

@tbouffard
Copy link
Member

@tbouffard tbouffard commented Oct 20, 2021

Remove bpmnVisualization package install and load: this has already been explained, so this is implicit.
Always use the bpmnVisualization scope when calling functions to ensure no ambiguity

@tbouffard tbouffard added the documentation Improvements or additions to documentation label Oct 20, 2021
@tbouffard tbouffard requested a review from csouchet October 20, 2021 14:19
ui <- shinyUI(fluidPage(
titlePanel("Display bpmn diagrams with execution data"),
bpmnVisualizationOutput('bpmnContainer')
bpmnVisualization::bpmnVisualizationOutput('bpmnContainer')
Copy link
Member Author

Choose a reason for hiding this comment

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

❓ not sure that a change is needed as the bpmnVisualizationOutput function is probably only provided by our package regarding its name.

Copy link
Contributor

Choose a reason for hiding this comment

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

When I wrote my article, I tested exactly this code: https://gist.github.com/csouchet/39a1d93fd28a78070b88050669fb4204#file-shiny-app-r
And it works without the bpmnVisualization::.
So we can keep as it is 🙂

server = function(input, output) {
# renderBpmnVisualization is the R bridge function to the html widgets
output$bpmnContainer <- renderBpmnVisualization({ displayBpmn() })
output$bpmnContainer <- bpmnVisualization::renderBpmnVisualization({ displayBpmn() })
Copy link
Member Author

Choose a reason for hiding this comment

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

❓ same here with renderBpmnVisualization

Copy link
Contributor

Choose a reason for hiding this comment

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

Same

@csouchet csouchet merged commit 6a70b7d into main Nov 8, 2021
@csouchet csouchet deleted the doc/improve_readme_shiny_app branch November 8, 2021 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants