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.

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

refactor: [POC] investigate replacement of mxGraph by [email protected] (2nd attempt) #2756

Closed
wants to merge 119 commits into from

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Jul 4, 2023

This is a POC and it is not intended to be merged into master, so don't expect to find a high quality code here! 😺
This is a continuation of #2366

Please don't rebase on master. If necessary, merge the master branch but instead we will create a new POC with the latest changes from the master branch.

Start from master e8fa907 (0.37.0 with dev dependencies update only)

Purpose

  • evaluate the migration effort
  • detect problems early
  • apply improvements to the master branch to ease the future migration and improve tests
  • provide feedback to the maxGraph community and if possible fixes that we integrated in this PR as workaround
  • be ready to easily reuse this effort in a new Pull Request to test new maxGraph releases

Status

This POC is completed.

  • rebase previous POC on master branch e8fa907 (post 0.37.0 version)
  • source compile
  • dev server starts, the demo is usable
    • label position OK (wasn't working in the previous POC), navigation and theme switch OK (see video below)
    • elements-identification demo works. It uses overlays, CSS and Style update (see video below)
  • ✔️ unit tests pass 💪🏿
  • ✔️ integration tests 🎉
  • ❌ e2e tests: they detect real issues.
    • Some BPMN rendering tests fails as in the previous POC due to issues in maxGraph 0.1.0 which are fixed in later versions.
      • message flow icon not correctly positioned
      • edge marker are not filled
    • To investigate
      • edge: the arcSize seems larger than with mxGraph
      • association and message flow dashed: the dashed positions are not exactly in the same order as with mxGraph
    • apparently work (currently failed because of existing issues described above)
      • overlays
      • fit/zoom
      • pan
      • theme
  • ❌ SVG export KO as in the previous POC. No work done on this topic
  • Remaining tasks associated to the migration are marked in the code as // TODO [email protected]. Most tasks are related to changes to do with newer versions of maxGraph

Demo: navigation, theme and label position

See #2756 (comment)

PR_2756_2nd_poc_maxgraph-0.1.0_fc95b24_navigation_theme_ok.mp4

Demo: Overlays, CSS and Style Update API

PR_2756_2nd_poc_maxgraph-0.1.0_c17cb046_elements-identification_OK.mp4

Improvements ported to the master branch

Detected maxGraph issues

Remaining tasks

Handle TODO in the code

Preview Give feedback

CSS classes not applied with the API

Preview Give feedback

Update Style API: make it work

Preview Give feedback

Rework the tests

Preview Give feedback

@tbouffard tbouffard added poc 💫 Experimentation to discuss about future implementation. Not intended to be merged mxgraph integration Something involving mxGraph (be aware of EOL) labels Jul 4, 2023
tbouffard added 28 commits July 4, 2023 06:53
@tbouffard
Copy link
Member Author

tbouffard commented Apr 22, 2024

POC completed, I am going to create a new poc based on the same commit on the master branch and maxGraph v0.10.0 to validate that this new version of maxGraph provides most improvements we need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mxgraph integration Something involving mxGraph (be aware of EOL) poc 💫 Experimentation to discuss about future implementation. Not intended to be merged WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant