Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add demo "visualize path between two consecutive shapes" #452

Merged
merged 36 commits into from
Feb 20, 2023
Merged

feat: add demo "visualize path between two consecutive shapes" #452

merged 36 commits into from
Feb 20, 2023

Conversation

csouchet
Copy link
Member

@csouchet csouchet commented Feb 8, 2023 •
edited by tbouffard
Loading

Here are the ways to use this demo.

  • Firstly, the user click on a shape (not an end event), then the shape is highlight, and put in gray all other shapes & edges.
  • Secondly, the user can highlight with another color the possible next path, on over on another shape.
  • Thirdly, the user click on a consecutive shape of the selected shape. The edge between the 2 shapes and the second shape are highlight.
Enregistrement.de.l.ecran.2023-02-14.a.17.11.19.mov

OR

  • Firstly, the user can highlight the possible path, on over.
  • Secondly, the user click on an edge, then the source and target shapes and the edge are highlight with another color, and put in gray all other shapes & edges.
Enregistrement.de.l.ecran.2023-02-14.a.17.11.48.mov

OR

  • Firstly, the user click on a shape (not an end event), then the shape is highlight, and put in gray all other shapes & edges.
  • Secondly, the user can highlight with another color the possible next path, on over on another edge.
  • Thirdly, the user click on a consecutive edge of the selected shape. The edge and the target shape of the edge are highlight.
Enregistrement.de.l.ecran.2023-02-14.a.17.11.34.mov

After a path is all highlight, if the user click on:

  • a third shape: The diagram is reset. The third shape becomes the first selected shape and it is highlight.
Enregistrement.de.l.ecran.2023-02-14.a.17.12.14.mov
  • a edge: The diagram is reset. The path of the edge is highlight. The source becomes the first selected shape, and the target becomes the second selected shape.
Enregistrement.de.l.ecran.2023-02-14.a.17.16.10.mov
Enregistrement.de.l.ecran.2023-02-14.a.17.13.03.mov

Live environment of examples

https://cdn.statically.io/gh/process-analytics/bpmn-visualization-examples/433-Add_a_new_demo_to_visualize_the_path_between_two_consecutive_shapes/examples/index.html

Closes #433

@csouchet csouchet added the enhancement New feature or request label Feb 8, 2023
@csouchet csouchet force-pushed the 433-Add_a_new_demo_to_visualize_the_path_between_two_consecutive_shapes branch from 051a416 to 07ffada Compare February 8, 2023 12:56
@tbouffard
Copy link
Member

tbouffard commented Feb 9, 2023 •
edited
Loading

First feedback on commit d26cd4a

Preview environment: https://cdn.statically.io/gh/process-analytics/bpmn-visualization-examples/d26cd4a/demo/draw-path/index.html

image

I like this demo 💯 I understood it purpose and how to use it at a glance 👍🏿
I didn't look at the code, I just wanted to see it live. AFAIK, the implementation is not completed, I will check it when it is ready for review. Please let me know.

Kudos for the legend which helps a lot. About the chosen colors, it seems to me that they lack contrast. To be check with an appropriate accessibility tool.

The elements of the breadcrumb contains links but they are not linking somewhere. Can we remove the links?
The "reset" button miss a title/popover.
The focus order of the elements in the breadcrumb is correctly configured. It is possible to navigate from the start element to the reset button.

PR_452_commit_d26cd4a_breadcrumb_links.mp4

Wording

Demo name
I don't know if it uses the final name, but I am not sure that "draw path" is the best name for this demo. I currently don't have any alternative to propose but I will think about it.

Shape/Edge
Are we wanted to hightlight the graphical or the semantic names? Currently, we are using the graphical wording.

Text in the description
The wording is currently using "impossible" looks very negative and only focus on what could be improve. I suggest we focus on what works instead.

image

@assynour assynour mentioned this pull request Feb 10, 2023
Open
@assynour
Copy link

assynour commented Feb 10, 2023 •
edited
Loading

I love the elegance of the layout. Minor suggestion: The user should not be allowed to click on the end event as it blocks the demo
path

@csouchet
Copy link
Member Author

csouchet commented Feb 14, 2023 •
edited
Loading

  • The link problem on the breadcrumb is fixed.
  • The problem for the first click on a end event is fixed too 🙂

@csouchet csouchet changed the title [DEMO] Add a new demo to visualize the path between two consecutive shapes feat: Add a new demo to visualize the path between two consecutive shapes Feb 14, 2023
@csouchet csouchet force-pushed the 433-Add_a_new_demo_to_visualize_the_path_between_two_consecutive_shapes branch from e4bfafc to fa2e9f1 Compare February 14, 2023 14:53
@csouchet csouchet force-pushed the 433-Add_a_new_demo_to_visualize_the_path_between_two_consecutive_shapes branch from 97b8fe5 to f93fab6 Compare February 14, 2023 16:10
@csouchet csouchet marked this pull request as ready for review February 16, 2023 13:34
@csouchet csouchet requested a review from tbouffard February 16, 2023 13:34
@tbouffard tbouffard changed the title feat: Add a new demo to visualize the path between two consecutive shapes feat: add a new demo to visualize the path between two consecutive shapes Feb 16, 2023
tbouffard
tbouffard reviewed Feb 16, 2023
View reviewed changes
demo/draw-path/js/path-use-case.js Outdated Show resolved Hide resolved
demo/draw-path/js/path-use-case.js Outdated
}

_getAllShapes() {
return this._bpmnVisualization.bpmnElementsRegistry.getElementsByKinds([
Copy link
Member

Choose a reason for hiding this comment

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

thought (non-blocking): Notice that in the past, we advised users to not get all elements using this API because it also does a lot DOM queries. This is also why we don't document how to do it in the JSDoc of the getElementsByKinds method.
However, I don't know if we can change the implementation in this demo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, we can find another way to get the shapes in another PR in order to not block the future implementation 🙂

demo/draw-path/js/path-use-case.js Outdated Show resolved Hide resolved
demo/draw-path/index.html Show resolved Hide resolved
demo/draw-path/index.html Outdated Show resolved Hide resolved
@csouchet csouchet requested a review from tbouffard February 16, 2023 16:41
tbouffard
tbouffard approved these changes Feb 20, 2023
View reviewed changes
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, great demo
✔️ checked with https://cdn.statically.io/gh/process-analytics/bpmn-visualization-examples/5a823b0/examples/index.html#demos

@tbouffard tbouffard changed the title feat: add a new demo to visualize the path between two consecutive shapes feat: add demo "visualize path between two consecutive shapes" Feb 20, 2023
@tbouffard tbouffard merged commit a341295 into master Feb 20, 2023
@tbouffard tbouffard deleted the 433-Add_a_new_demo_to_visualize_the_path_between_two_consecutive_shapes branch February 20, 2023 10:13
@tbouffard tbouffard mentioned this pull request Feb 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbouffard tbouffard tbouffard approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DEMO] Add a new demo to visualize the path between two consecutive shapes
3 participants
@csouchet @tbouffard @assynour