Skip to content

Implement edit capabilities for architecture diagram visualizer#1019

Closed
msdorothyewuah wants to merge 24 commits intofinos:mainfrom
msdorothyewuah:calm-ui-feature-779
Closed

Implement edit capabilities for architecture diagram visualizer#1019
msdorothyewuah wants to merge 24 commits intofinos:mainfrom
msdorothyewuah:calm-ui-feature-779

Conversation

@msdorothyewuah
Copy link

@msdorothyewuah msdorothyewuah commented Mar 14, 2025

Architecture Diagram Editor

This PR implements the initial functionality to convert our read-only architecture diagram visualizer into an interactive editor. This allows users to add nodes and edges to the diagram and save the updated JSON.

Currently Implemented:

  • Shell[new] node concept with distinctive styling (dashed border)
  • Add Node button creates a new shell node in the diagram
  • Sidebar integration for editing node details
  • JSON export
  • Data persistence when editing existing nodes and edges

In Progress:

  • Edge creation functionality from shell nodes
  • Button positioning and UI improvements
  • Data persistence when editing newly created nodes

Known Issues:

  • Saving details for newly created nodes doesn't update the node display
  • Edge creation from plus buttons not yet fully functional

This PR is being submitted as WIP to get early feedback on the implementation approach while continuing to work on the remaining issues.

Screenshots attached below show current progress.

Screenshot 2025-03-14 at 1 27 49 PM Screenshot 2025-03-14 at 1 28 06 PM Screenshot 2025-03-14 at 1 28 34 PM

WIP: Implement edit capabilities for architecture diagram visualizer
@oliviajanejohns
Copy link
Contributor

To do:

  • adding nodes
  • adding a new edge and connecting
  • styling / re-positioning UX

@github-actions github-actions bot removed the config label Mar 14, 2025
@oliviajanejohns
Copy link
Contributor

Noticed a couple of bugs that just looking in to:

  • When editing, the side panel no longer covers the height of the page
    Screenshot 2025-03-16 at 21 36 35

  • Additionally, noticed that when selecting 'add node' the added node only appears after clicking on the graph

@jpgough-ms jpgough-ms marked this pull request as draft March 20, 2025 08:25
@jpgough-ms
Copy link
Member

@msdorothyewuah please mark this PR as ready when builds are working and you are ready for this to be reviewed

jpgough-ms and others added 2 commits March 20, 2025 08:25
The editor now provides a complete flow for creating/editing the architecture diagram
You can now:
1. Add nodes with the "Add Node" button
2. Edit node/edge details in the sidebar
3. Create connections between nodes from the sidebar
4. Export the resulting diagram as JSON
5. I included a delete to be able to remove newly added nodes or edges.
@msdorothyewuah msdorothyewuah marked this pull request as ready for review March 20, 2025 12:45
@msdorothyewuah
Copy link
Author

This PR is ready for review.

The editor now provides a complete flow for creating/editing the architecture diagram.

You can now:

  • Edit node/edge details in the sidebar
  • Create connections between nodes from the sidebar
Screenshot 2025-03-20 at 12 39 48 PM
  • Add nodes with the "Add Node" button
  • Export the resulting diagram as JSON
  • Remove newly added nodes or edges.
Screenshot 2025-03-20 at 12 40 06 PM

@jpgough-ms
Copy link
Member

@msdorothyewuah lint is failing, please can you take a look

@msdorothyewuah msdorothyewuah changed the title WIP: Implement edit capabilities for architecture diagram visualizer Implement edit capabilities for architecture diagram visualizer Mar 20, 2025
@oliviajanejohns
Copy link
Contributor

Currently seeing the below error:
Screenshot 2025-03-24 at 21 14 15

Seems that drawer is still in use - so not sure if it should be deleted!

@github-actions github-actions bot removed the config label Apr 8, 2025
@oliviajanejohns
Copy link
Contributor

PR out of date with latest changes

@msdorothyewuah
Copy link
Author

Screenshot 2025-04-14 at 9 46 47 PM

@oliviajanejohns @YoofiTT96 @aamanrebello @Adwoa-Konadu-Appiah as we discussed during our UI/UX sync on Friday, this is the full implementation, now complete with detailed documentation and resolution of merge and implementation conflicts.

label: '', // No native label
width: '200px',
height: 'label',
height: isNodeDescActive ? '130px' : '70px',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 130px an arbitrary height? What if the description is longer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can be tackled in another PR though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this as the average height while we figure out a better approach

group: 'nodes',
classes: 'node shell-node',
data: shellNodeData,
position: { x: 300, y: 300 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we always adding new nodes in a fixed position? Again, in a future PR, we may want to improve this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you add multiple? Are they all added on top of each other?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can only create one shell node at a time as you are required to fill in the node details before you proceed but no when you create two they do not stack up on each other

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the diagram is complicated it could be good to know where the shell was added.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I will note this down for future iterations

@aamanrebello
Copy link
Contributor

Do you encounter this problem when selecting a new node?

image

@msdorothyewuah
Copy link
Author

msdorothyewuah commented Apr 15, 2025

Do you encounter this problem when selecting a new node?

Screenshot 2025-04-15 at 9 27 32 AM

@aamanrebello I’m not seeing this issue on my end, but it looks like the double rendering bug we discussed is showing up in complex diagrams. Noted. Definitely something to keep in mind as we finalize the rendering approach

@oliviajanejohns
Copy link
Contributor

Whats the latest with the bug mentioned above?

@msdorothyewuah
Copy link
Author

Closing this PR following the discussion to hold off on adding editing capabilities for now. Happy to revisit it in the future if priorities change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants