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

[BUG] Failed to execute 'getComputedStyle' when using lit #2738

Closed
xxxLukskyxxx opened this issue Jun 8, 2023 · 6 comments · Fixed by process-analytics/bpmn-visualization-examples#507
Labels
bug Something isn't working
Milestone

Comments

@xxxLukskyxxx
Copy link

Describe the bug
When the cursor is moved over the container, an error is triggered:
Uncaught TypeError: Failed to execute 'getComputedStyle' on 'Window': parameter 1 is not of type 'Element'.

To Reproduce

  1. Setup project with typescript, lit and vite
  2. Use the bpmn-visualization in a custom element

Screenshots
Error message in Chrome:
image
Error message in Firefox:
image

Desktop (please complete the following information):

  • Windows 10
  • Chrome, Firefox, Edge
  • Chrome 114.0.5735.90, Firefox 114
@xxxLukskyxxx xxxLukskyxxx added the bug Something isn't working label Jun 8, 2023
@brendanlaschke
Copy link
Contributor

It seems to be related to the shadow dom of lit-element and the way mxGraph detects offsets. The Dom is traversed upwards (node = node.parentNode;) until a

  • position: fixed node is reached
  • or document.body is reached
  • or document.documentElement is reached
  • or the parent Node null is

None of them is the case for the shadow dom root of the web component -> the window.getComputedStyle is called on the shadow dom root and fails.

An obvious fix (if possible in your usage context) is to use a container with position: fixed around the bpmn-container. But this is no general fix as this influences positioning.

An reproduction example: https://codesandbox.io/p/sandbox/brave-curie-jhzvd9

But I don't have an idea at the moment how to change the behavior without touching mxGraphs code.

@tbouffard
Copy link
Member

tbouffard commented Jun 9, 2023

Thanks for the detailed report and the reproduction environment.
The problem seems to be related to a mxGraph issue: jgraph/mxgraph#186
As mxGraph is discontinued, we won't get a fix for that.

However, I know that someone fixed (or at least pretended to fix) this issue:

A fix is available in aire-ux/mxgraph#1: this is a very large Pull Request containing other topics. The fix in located in mxUtils.getCurrentStyle and is the one proposed in jgraph/mxgraph#186

You may use something like patch-package to locally patch mxGraph with the fix mention above.

tbouffard added a commit to process-analytics/bpmn-visualization-examples that referenced this issue Jun 9, 2023
Use patch-package to patch mxGraph and fix process-analytics/bpmn-visualization-js#2738.
Built with vite.
@tbouffard
Copy link
Member

tbouffard commented Jun 9, 2023

I have been able to make it work by patching/fixing mxGraph with patch-package in a dedicated example: process-analytics/bpmn-visualization-examples#507
It is based on the codesandbox project provided in #2738 (comment).

@xxxLukskyxxx Please let me know if this could work for you.

@tbouffard
Copy link
Member

After discussion with @csouchet, we decided to not fix mxGraph directly in bpmn-visualization.
process-analytics/bpmn-visualization-examples#507 demonstrates how to workaround the problem.

We recognize that this problem has an impact on applications that want to integrate bpmn-visualization into Web Components. However, fixing mxGraph directly may introduce side-effects for other types of integration, whereas Web Components are not the most common use of bpmn-visualization.
The root cause of the problem lies in mxGraph, which is no longer maintained. We're looking into the possibility of switching to a new diagram library.
maxGraph is a candidate (see #2366) and will correct this problem (maxGraph/maxGraph#68). We may also decide to switch to another library such as https://github.com/antvis/X6.
So in a few months' time, you can expect this problem to be handled entirely by bpmn-visualization.

If you need more information, don't hesitate to post a comment here.

@xxxLukskyxxx
Copy link
Author

@tbouffard Thank you for the fast patch!

@tbouffard
Copy link
Member

Perfect. I would like to thanks @brendanlaschke again for the analysis and the reproduction environment.

The analysis enabled me to quickly recall the existing mxGraph problem. The associated issue proposed a workaround that I was able to use.
I used the reproduction environment to validate the workaround and all that remained was to update it using a more recent version of the development dependencies 😸.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants