Smart Connector UIExtension #308

yentelmanero · 2023-11-30T00:51:55Z

This PR is part of my bachelor research conducted in TU Wien in the Business Informatics Group.

In this PR, the Smart Connector UIExtension, which is a Tool that hovers around the selected node, simplifying the creation of new nodes and edges, was created.

Here are some screenshots of the UIExtension:

(different types of nodes can have different options)

Server PR
Theia PR

…t, right, up, down)

…connector

…nnector

haydar-metin

Thanks for the PR! I did just a preliminary look through the code. After the server-side compiles correctly, I will try it out and look deeper into the code.

Can you also add some description to your PR and reference the other PRs you have created, something like:

This PR is part of my bachelor research conducted in TU Wien in the [Business Informatics Group](https://www.big.tuwien.ac.at/).

---

<Little Description>

<Other PR references>

packages/client/src/features/smart-connector/smart-connector-module.ts

packages/client/src/utils/html-utils.ts

packages/protocol/src/action-protocol/smart-connector.ts

packages/client/src/features/smart-connector/smart-connector.ts

haydar-metin · 2023-12-19T13:23:59Z

packages/client/src/features/smart-connector/smart-connector.ts

+            this.createTool(item);
+        }
+        if (event.ctrlKey) {
+        // matchesKeystroke with Ctrl and Ctrl+F does not seem to work on Windows 11/Chrome


Reminder to find out why afterward

yentelmanero · 2023-12-27T15:39:44Z

Thanks for the PR! I did just a preliminary look through the code. After the server-side compiles correctly, I will try it out and look deeper into the code.

Can you also add some description to your PR and reference the other PRs you have created, something like:
This PR is part of my bachelor research conducted in TU Wien in the [Business Informatics Group](https://www.big.tuwien.ac.at/).

---

<Little Description>

<Other PR references>

@haydar-metin, unfortunately, the server cannot build because of the missing packages from this PR. Is there a way to circumvent this issue?

haydar-metin

Thank you very much, I just tested it and reviewed the code. Largely the code looks good. There are some small requests from me. Functionality wise it looks very good! Just small remarks.

Can you also add the finished solution as an image to the PR? Similar to the images provided here.

Styling

I think visually it would be more appealing if the boxes would have a top and bottom border. Otherwise it looks like there is more.

Usage

I think in those cases nothing should be shown?

We are almost there! Good job!

packages/protocol/src/action-protocol/smart-connector.ts

packages/client/src/features/smart-connector/smart-connector.ts

merge newest to fork

… change Smart Connector state, added top and bottom borders to Smart Connector

haydar-metin · 2024-02-08T11:52:30Z

UI

You probably need to add a background. .smart-connector-group-container is transparent
The arrow is not correctly aligned
Scrollbar is not visible - in the CSS you want to show it but is not shown

Can you add a listener for Escape that closes the smart connector

Other

Please resolve all the comments I have written and which are already finished (I can not to that)
Please install Prettier and run the auto format for all of your files
I will then notify someone else and then we can probably merge it if everything works as intended

yentelmanero · 2024-02-08T16:50:38Z

UI

You probably need to add a background. .smart-connector-group-container is transparent

The arrow is not correctly aligned

Scrollbar is not visible - in the CSS you want to show it but is not shown

Can you add a listener for Escape that closes the smart connector

Other

Please resolve all the comments I have written and which are already finished (I can not to that)

Please install Prettier and run the auto format for all of your files

I will then notify someone else and then we can probably merge it if everything works as intended

Hi, how are you running the project? I am running the client and then the theia integration and there are no issues here as you can see in a similar image:

haydar-metin · 2024-02-09T08:20:35Z

Hey, I also tested the standalone version, that means you need to build the client with yarn and start your node server with yarn start -w -p 8081. Then you need to open the file: file:///.../glsp-client/examples/workflow-standalone/app/diagram.html in your browser. I think you just need to move some of your CSS there. (PS: You need to recompile the workflow-standalone after changes)

https://github.com/eclipse-glsp/glsp-client?tab=readme-ov-file#how-to-start-the-workflow-diagram-example

…, fixed css issues

yentelmanero · 2024-02-12T00:34:13Z

packages/client/src/features/smart-connector/smart-connector.ts

+            this.hideSmartConnector();
+            // assumes that the graph is always the last child of base div
+            // this focus is done to "reactivate" the key listener to re-open if needed
+            document.getElementById(this.options.baseDiv)?.last().focus();


Since the focus is lost when the connector is closed, the KeyListener does not react to any inputs. It does also not react when any tool is selected. Because of this, I just added a onkeydown event on every tool and when it closes, it refocuses on the base div (the graph). Is this assumption here correct? Or is there a better way to get the base div?

I'm not sure what you mean, why is it bad that the focus is lost when you hide the connector?

Because the KeyListener does not react anymore. So if I, for example, open the connector with space, close it again with escape and then try to open it again, it won't work

yentelmanero · 2024-02-16T13:30:04Z

@haydar-metin Hi, is there any information regarding the status of the PR? Are there any more changes to be done?

haydar-metin · 2024-02-16T14:45:21Z

@yentelmanero I requested a second review internally. After that, we can merge it.

…d node

tortmayr

In general the code looks good to me and the SmartConnector seems to work really well.
I have a couple of inline comments/suggestions.

As we discussed offline we would prefer to drop the concept of SmartConnector in favor of a more generic SelectionPalette and SelectionPaletteProvider API.

Please rename the corresponding artifacts accordingly.

Also there seems to be some room for improvements regarding the positing of the smartconnector when moving/resizing a node and scrolling/zooming:

Scrolling/zooming: When scrolling or zooming the smart connector gets hidden although the node is still selected
Resize: The position of the + button is not recentered when resizing
Moving: The smart connector gets hidden during move (which is fine imo) but is not set visible again once the move is finished. (Node is still selected)

However, these are issue that can be addressed in a follow-up PR and don't necessarily have to be covered by you.

packages/client/src/default-modules.ts

packages/client/src/features/smart-connector/smart-connector.ts

packages/client/src/utils/htmlelement-extensions.ts

packages/protocol/src/action-protocol/smart-connector.ts

…actions for selection palette (are now handled as onSelectionChanged listener), moved SelectionPaletteState and action to new file, removed HTMLElement extensions

…ng anymore

yentelmanero · 2024-04-02T17:19:12Z

Hi @tortmayr, thanks for the feedback.

I changed the code snippets as requested and also added that the selection palette does not disappear when panning and zooming, since that changed seemed simple enough to do it relatively quickly.

Since I changed quite a bit of code there, because I am not using the DOM measurements anymore but the Viewport and GNode data, I would ask you to please review those parts once again. The reason I changed that is because element.getBoundingClientRect() was too slow in delivering the needed information, so I would calculate with the old frame, while already showing a new frame, resulting in wrong behaviour when zooming.

I am also not entirely sure if the way I added the explicit module imports (for both workflow-standalone and workflow-theia) was done correctly, but it worked after I added those and it did not before.

…disappearing

yentelmanero · 2024-04-18T17:52:50Z

Hi @tortmayr, thanks for the feedback.

I changed the code snippets as requested and also added that the selection palette does not disappear when panning and zooming, since that changed seemed simple enough to do it relatively quickly.

Since I changed quite a bit of code there, because I am not using the DOM measurements anymore but the Viewport and GNode data, I would ask you to please review those parts once again. The reason I changed that is because element.getBoundingClientRect() was too slow in delivering the needed information, so I would calculate with the old frame, while already showing a new frame, resulting in wrong behaviour when zooming.

I am also not entirely sure if the way I added the explicit module imports (for both workflow-standalone and workflow-theia) was done correctly, but it worked after I added those and it did not before.

Update: now also added the selection palette moving when resizing or moving nodes, since I could not find an explanation why I did not do it when writing my thesis. Please review

tortmayr · 2024-04-24T10:25:10Z

@yentelmanero
Thank your for this awesome contribution.
I have reviewed and tested the changes. Everything seems to work as expected and all review comments have been addressed.
Nice work!

We will keep this PR open and merge it after the 2.2.0 Release is done (end of May). This way we have a full release cycle to polish the feature and address follow-up issues before we are shipping it to adopters.

In any case, from our side this concludes the practical aspect of your bachelor thesis. We don't require/desire any more changes from your side. Congrats!

yentelmanero added 6 commits October 30, 2023 23:14

added smart connector UIExtension and Actions for data request

0307ab8

added possibility to set position for smart connector containers (lef…

7efe329

…t, right, up, down)

Merge remote-tracking branch 'upstream/master'

522284b

added search, keyboard navigation and customization options to smart …

b41978d

…connector

fixed keyboard, width and click event when invisible bugs in smart co…

7a0e67a

…nnector

Merge remote-tracking branch 'upstream/master'

3c3bcad

yentelmanero changed the title ~~glsp-client merge smart connector~~ Smart Connector UIExtension Nov 30, 2023

yentelmanero added 2 commits November 30, 2023 13:09

addressed lint issues

99fc2d4

moved HTMLElement extension methods from smart-connector to html-utils

68a3430

haydar-metin reviewed Dec 19, 2023

View reviewed changes

This was referenced Dec 27, 2023

CSS for Smart Connector UIExtension eclipse-glsp/glsp-theia-integration#188

Open

Handlers for Smart Connector UIExtension eclipse-glsp/glsp-server-node#70

Open

yentelmanero added 3 commits December 27, 2023 15:26

Implemented suggested changes from PR

6c811fc

Merge remote-tracking branch 'upstream/master'

c4de64b

added index file for smart connector

fae839d

fixed lint issues

3405a06

haydar-metin reviewed Jan 10, 2024

View reviewed changes

yentelmanero and others added 2 commits January 30, 2024 15:28

Merge pull request #1 from eclipse-glsp/master

c9ae88e

merge newest to fork

Implemented PR changes, moved HTMLElement extensions, added Action to…

238cec9

… change Smart Connector state, added top and bottom borders to Smart Connector

yentelmanero and others added 3 commits February 12, 2024 01:13

Merge branch 'eclipse-glsp:master' into master

221602b

Added escape key to exit smart connector, applied prettier formatting…

8732697

…, fixed css issues

removed comment

001a16e

yentelmanero commented Feb 12, 2024

View reviewed changes

fixed bug where top and bottom containers would not align with relate…

12a1f77

…d node

yentelmanero added 2 commits February 25, 2024 17:03

fixed issue where icons could not be used for tool buttons

f7bafea

changed line to use display:flex instead of block after searching

defec53

tortmayr self-requested a review March 25, 2024 07:48

tortmayr requested changes Mar 25, 2024

View reviewed changes

yentelmanero added 5 commits April 1, 2024 17:44

renamed smart connector to selection palette, removed open and close …

1bcc513

…actions for selection palette (are now handled as onSelectionChanged listener), moved SelectionPaletteState and action to new file, removed HTMLElement extensions

Merge branch 'master' of https://github.com/yentelmanero/glsp-client

c88f126

fixed merge issues happening due to name change

b0774d7

selection palette expand button does not close when panning and zoomi…

3cc4e58

…ng anymore

fixed lint problems

152a9ff

yentelmanero added 2 commits April 16, 2024 12:15

fixed bug where edge selection would also bring up selection palette

b3ad8c7

selection palette now moves when resizing and moving node instead of …

b470530

…disappearing

yentelmanero mentioned this pull request Apr 18, 2024

added Selection Palette as experimental to readme eclipse-glsp/glsp#1321

Open

yentelmanero requested a review from tortmayr April 18, 2024 18:09

tortmayr approved these changes Apr 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smart Connector UIExtension #308

Smart Connector UIExtension #308

yentelmanero commented Nov 30, 2023 •

edited

Loading

haydar-metin left a comment

haydar-metin Dec 19, 2023

yentelmanero commented Dec 27, 2023

haydar-metin left a comment •

edited

Loading

haydar-metin commented Feb 8, 2024

yentelmanero commented Feb 8, 2024

UI

Other

haydar-metin commented Feb 9, 2024 •

edited

Loading

yentelmanero Feb 12, 2024

haydar-metin Feb 16, 2024

yentelmanero Feb 16, 2024

yentelmanero commented Feb 16, 2024

haydar-metin commented Feb 16, 2024

tortmayr left a comment

yentelmanero commented Apr 2, 2024 •

edited

Loading

yentelmanero commented Apr 18, 2024

tortmayr commented Apr 24, 2024

Smart Connector UIExtension #308

Are you sure you want to change the base?

Smart Connector UIExtension #308

Conversation

yentelmanero commented Nov 30, 2023 • edited Loading

haydar-metin left a comment

Choose a reason for hiding this comment

haydar-metin Dec 19, 2023

Choose a reason for hiding this comment

yentelmanero commented Dec 27, 2023

haydar-metin left a comment • edited Loading

Choose a reason for hiding this comment

Styling

Usage

haydar-metin commented Feb 8, 2024

UI

Other

yentelmanero commented Feb 8, 2024

UI

Other

haydar-metin commented Feb 9, 2024 • edited Loading

yentelmanero Feb 12, 2024

Choose a reason for hiding this comment

haydar-metin Feb 16, 2024

Choose a reason for hiding this comment

yentelmanero Feb 16, 2024

Choose a reason for hiding this comment

yentelmanero commented Feb 16, 2024

haydar-metin commented Feb 16, 2024

tortmayr left a comment

Choose a reason for hiding this comment

yentelmanero commented Apr 2, 2024 • edited Loading

yentelmanero commented Apr 18, 2024

tortmayr commented Apr 24, 2024

yentelmanero commented Nov 30, 2023 •

edited

Loading

haydar-metin left a comment •

edited

Loading

haydar-metin commented Feb 9, 2024 •

edited

Loading

yentelmanero commented Apr 2, 2024 •

edited

Loading