Allow multiple diagrams to be opened #44

martin-fleck-at · 2023-11-30T14:30:37Z

Fix TheiaJsonrpcGLSPClient to only have one handler for connection
Fix problem with filter with same ID being added multiple times
Add formatting of CSS files to prettier
Centralize ID handling for containers and global IDs -- Diagram elements need locally unique IDs
-- Other elements need globally unique IDs

Fixes #16

- Fix TheiaJsonrpcGLSPClient to only have one handler for connection - Fix problem with filter with same ID being added multiple times - Add formatting of CSS files to prettier - Centralize ID handling for containers and global IDs -- Diagram elements need locally unique IDs -- Other elements need globally unique IDs Fixes #16

github-actions · 2023-11-30T14:44:33Z

Unit Test Results

    3 files ±0   30 suites ±0 2m 21s ⏱️ -6s
  68 tests ±0   68 ✔️ ±0 0 💤 ±0 0 ❌ ±0
207 runs ±0 207 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 4dcdc37. ± Comparison against base commit 76c7430.

♻️ This comment has been updated with latest results.

…so tests fail on actions i.s.o. timeout.

harmen-xb

Hi Martin,

Thanks for these changes. Codewise I don't see strange things.

I do see the PlayWright tests timed-out, so I updated the playwright config so it will really error on actions which take to long (like waitFor* actions). Otherwise it might end-up unnoticed.

While testing I experiences some issues. I wrote the issues in separate comments below. The first issue I related to this branch. The other two I am not sure, so I can also move them into separate issues to be solved later (depends on the time it costs to fix it).

harmen-xb · 2023-11-30T22:28:32Z

Issue 1: Couldn't find node

Also I experience an error when modeling a new diagram, where I get an error that it couldn't find a node with the given id:

2023-11-30T21:57:46.157Z root ERROR Request requestDiagramNodeEntityModel failed with error: Request server/requestModelDiagramNode failed with message: No node found with the given id: CustomerToOrder Error: Request server/requestModelDiagramNode failed with message: No node found with the given id: CustomerToOrder
    at handleResponse (/workspaces/crossmodel/node_modules/vscode-jsonrpc/lib/common/connection.js:565:48)
    at handleMessage (/workspaces/crossmodel/node_modules/vscode-jsonrpc/lib/common/connection.js:345:13)
    at processMessageQueue (/workspaces/crossmodel/node_modules/vscode-jsonrpc/lib/common/connection.js:362:17)
    at Immediate.<anonymous> (/workspaces/crossmodel/node_modules/vscode-jsonrpc/lib/common/connection.js:334:13)
    at processImmediate (node:internal/timers:466:21)

Steps to reproduce:

Open the yaml-example workspace
Create a new diagram
Drag an the two existing entities.
Create a new relationship using the GLSP Toolbox between the two entities.
Now click on the relationship in the diagram, then the error is in the terminal.

When I look in the diagram code I see the following contents:

    edges:
      - id: "CustomerToOrder"
        relationship: "unknown/CustomerToOrder"
        sourceNode: "CustomerNode"
        targetNode: "OrderNode"

When I compare this with the other diagram which is working, the unknown/ seems to be the issue. So it looks like local ids are not created correctly yet. When I look at the contents of the new relationship I also see the two entities are references prefixed with unknown/.

When I remove the unknown/ in both files, I still get the error.

harmen-xb · 2023-11-30T22:29:54Z

Issue 2: Diagram context menu issue

I also have another issue with the context menu re-occurring.
Sometimes when I right-click a diagram node in the explorer view it just misses a lot of option. See for example here:

If I then click on another node, and back and then right-click it doesn work agaain:

We had this issue before on entities, but now I can also reproduce it on diagram files (which seems to be related to a diagram being opened).

What I also noticed that when the context menu is wrong the keyboard actions also don't work. Like deleting the file with the 'Delete' key doesn't work, this is also not in the context menu if it's wrong. So I guess this is linked

Is this a known Theia/GLSP issue?

Steps to reproduce:

Open the yaml-example workspace
Make sure no diagram is open
Click on a diagram so it opens
Right-click on the diagram file to open the context menu.

It this point the context menu doesn't contain all options. When I choose the 'Copy' option and paste it in a text editor I get the full path of the diagram. So it does seem to execute the action on the right node.

Once you have the diagram open, and leave it open the problem doesn't re-occur when working with different files and jumping back. So it seems related to the diagram tab being opened.

harmen-xb · 2023-11-30T22:36:35Z

Issue 3: Property widget starts flashing (yes you read it correctly 😄 )

At some point I saw the property widget started flashing, looks like it is re-rendering continuously.

Steps to reproduce:

Open the yaml-example workspace
Make sure the property view is visible
Open the existing diagram
Make a new diagram and drag the Customer entity into it.
Click on the Customer entity

At this point on Chrome Windows the property widgets kind of starts flashing, where the contents looks to be reloaded every second.

Once you go to the other diagram and click on an entity it works fine. If you then jump back to the new diagram and click on an entity it also works fine.

martin-fleck-at · 2023-12-01T08:42:28Z

requestDiagramNodeEntityModel

Yes, I think that probably never worked. I just checked the code and it comes from the property view that requests an entity but when you click on an edge the server rightfully says that there is no node with the given ID because we clicked on an edge. I'll see what we can do to best prevent that.

Problem: We open an entity through the property view. That triggers a rebuild of the entity document which triggers an update in all open diagrams that have that entity as they have a reference to it. The diagram update also resets the selection state in each diagram which in turn updates the property view again as it reacts to selection changes. This can lead to an update-cycle if there is more than one diagram open with the same entity. Fix: Since the property view reacts to selection changes and updates in the diagram trigger such a change, there is no need for the property view to listen actually participate in the model lifecycle itself. So there is no need to do the open/close since we do not need to listen to updates ourselves.

- Ensure that the navigator tree always gets a URI of the selected node

harmen-xb · 2023-12-01T13:58:04Z

Hi @martin-fleck-at,

I see you pushed some changes to fix the issues.

I tested it and all issues indeed seem to be solved.

A new issue I now see is that the property widget form is working with multiple diagrams open. But when having two diagrams open and change something using the property widget and press Save the entity file is updated, but then the property widget is emptied and the diagram is not updated. So something is not OK yet.

martin-fleck-at · 2023-12-01T14:06:33Z

@harmen-xb Interesting, I'll double check that, thank you for the quick review!

- Ensure GLSP selection is forwarded again if active editor changes - Only update properties if selection of active widget changes - Add additional change reason for better update decision in GLSP

martin-fleck-at · 2023-12-03T19:45:03Z

@harmen-xb I pushed another update that should fix that issue. Could you please have another look?

Added context menu tests on relationship and diagram files in the explorer view.

harmen-xb · 2023-12-08T09:50:27Z

@martin-fleck-at Thanks for the update! It now all seems the work correctly.

To make sure the context menu's keep working I added some extra playwright tests on the context menu of relationship and diagrams.

harmen-xb

All looks ok now.

martin-fleck-at requested a review from harmen-xb November 30, 2023 14:47

harmen-xb added 2 commits November 30, 2023 21:43

Updated playwright config to have timeouts for action and navigation …

409b5cc

…so tests fail on actions i.s.o. timeout.

Added logging of the id of the node it couldn't find.

3ce5e47

harmen-xb requested changes Nov 30, 2023

View reviewed changes

martin-fleck-at added 3 commits December 1, 2023 14:18

Fix "Couldn't find node" when selecting something other than an entity

1b74139

Fix Diagram context menu issue

73aa596

- Ensure that the navigator tree always gets a URI of the selected node

Improve update handling between editors

9d084d1

- Ensure GLSP selection is forwarded again if active editor changes - Only update properties if selection of active widget changes - Add additional change reason for better update decision in GLSP

martin-fleck-at force-pushed the feature/issue-16 branch from 877dcf2 to 9d084d1 Compare December 3, 2023 19:30

martin-fleck-at mentioned this pull request Dec 7, 2023

Enhance grammar to support attribute mapping from sources to target #45

Merged

Added example relationship and diagram to e2e-tests example worspace.

4dcdc37

Added context menu tests on relationship and diagram files in the explorer view.

harmen-xb approved these changes Dec 8, 2023

View reviewed changes

harmen-xb merged commit bf4c2c0 into main Dec 8, 2023
5 checks passed

harmen-xb deleted the feature/issue-16 branch December 8, 2023 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple diagrams to be opened #44

Allow multiple diagrams to be opened #44

martin-fleck-at commented Nov 30, 2023

github-actions bot commented Nov 30, 2023 •

edited

harmen-xb left a comment •

edited

harmen-xb commented Nov 30, 2023 •

edited

harmen-xb commented Nov 30, 2023 •

edited

harmen-xb commented Nov 30, 2023 •

edited

martin-fleck-at commented Dec 1, 2023

harmen-xb commented Dec 1, 2023 •

edited

martin-fleck-at commented Dec 1, 2023

martin-fleck-at commented Dec 3, 2023

harmen-xb commented Dec 8, 2023

harmen-xb left a comment

Allow multiple diagrams to be opened #44

Allow multiple diagrams to be opened #44

Conversation

martin-fleck-at commented Nov 30, 2023

github-actions bot commented Nov 30, 2023 • edited

Unit Test Results

harmen-xb left a comment • edited

Choose a reason for hiding this comment

harmen-xb commented Nov 30, 2023 • edited

Issue 1: Couldn't find node

Steps to reproduce:

harmen-xb commented Nov 30, 2023 • edited

Issue 2: Diagram context menu issue

Steps to reproduce:

harmen-xb commented Nov 30, 2023 • edited

Issue 3: Property widget starts flashing (yes you read it correctly 😄 )

Steps to reproduce:

martin-fleck-at commented Dec 1, 2023

harmen-xb commented Dec 1, 2023 • edited

martin-fleck-at commented Dec 1, 2023

martin-fleck-at commented Dec 3, 2023

harmen-xb commented Dec 8, 2023

harmen-xb left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 30, 2023 •

edited

harmen-xb left a comment •

edited

harmen-xb commented Nov 30, 2023 •

edited

harmen-xb commented Nov 30, 2023 •

edited

harmen-xb commented Nov 30, 2023 •

edited

harmen-xb commented Dec 1, 2023 •

edited