Skip to content

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 maxgraph@0.1.0. Most tasks are related to changes to do with newer versions of maxGraph

Bundle sizes comparison with commit 52f9c80

Bundle mxGraph 4.2.2 maxGraph 0.1.0
IIFE raw 1 669 729 1 466 189
IIFE minified 984 186 703 551
ESM (lib code only) 198 486 195 232
Demo chunk 818 KB 563.29 kB

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
- [x] manage the "TODO rebase" in the code. Created while rebasing the previous code on the master branch
- [x] review all "TODO maxgraph@0.1.0": some can be handled thanks to the fix done on the master branch and that weren't available in the previous POC
- [x] the extraCssClasses should be a property in the CellStyle and not defined with a deep nested object
### CSS classes not applied with the API
- [x] in the `elements-identifications.html` demo, the CSS are sometimes applied but seems to disappear during navigation
- [x] integration tests fail: the CSS classes are in the maxGraph model, but they are not generated in the DOM
### Update Style API: make it work
- [x] font style value is incorrect. Is is lower by one. Use workaround to fix the problem (seems to be an issue in maxGraph)
- [x] passing default value work
- [x] update and reset work
### Rework the tests
- [x] Make the tests pass
- [x] Provide a status of the e2e tests.

@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.

2 participants