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

chore: switch to visualiser library #342

Closed

Conversation

jonaslagoni
Copy link
Member

Description
This PR switches to the split-out visualization library.

Related issue(s)
fixes #261

@netlify
Copy link

netlify bot commented May 20, 2022

Deploy Preview for modest-rosalind-098b67 ready!

Name Link
🔨 Latest commit 921c8e9
🔍 Latest deploy log https://app.netlify.com/sites/modest-rosalind-098b67/deploys/6384c0528361e30009e1f808
😎 Deploy Preview https://deploy-preview-342--modest-rosalind-098b67.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@jonaslagoni jonaslagoni marked this pull request as ready for review May 25, 2022 15:23
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@magicmatatjahu
Copy link
Member

@jonaslagoni We probably should "wait" with this PR, due to simple reasons:

  • styles atm aren't good, you can see:

    image

  • we don't have "title" as we have in current implementation:

    image

  • we should have also "flow" animation as we have in current implementation and we should add buttons for it (in top right corner):

    image

  • we should have also additional buttons as we have in bottom left corner:

    image

I think that we should wait and merge it when we will move library to AsyncAPI org and fix above problems - of course PR can be still open :)

@magicmatatjahu
Copy link
Member

/dnm

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Sep 25, 2022
@fmvilas
Copy link
Member

fmvilas commented Sep 27, 2022

@jonaslagoni are you going to work on this? Do you need help?

@github-actions github-actions bot removed the stale label Sep 28, 2022
@jonaslagoni
Copy link
Member Author

@jonaslagoni are you going to work on this? Do you need help?

If you could give me 48 hours a day, that would be really helpful 😆

Current status if I am not mistaken:

  • Still waiting for Donating EDAVisualiser community#382 as we don't want to integrate it with Studio until then.
  • Need to look into the styling and if anything is missing from the implementation.
  • The buttons and titles that are missing are possible to add rather easily, so I just need to apply that.

Other then that, yes time is what I need 😆

@jonaslagoni
Copy link
Member Author

Alright, I fixed the issues you outlined @magicmatatjahu. I did not include any extra features as this is solely about switching internals with a library 🙂

Any extra stuff I would suggest we add later.

@jonaslagoni
Copy link
Member Author

we should have also "flow" animation as we have in current implementation and we should add buttons for it (in top right corner):

Okay I missed that one, that needs to be adapted.

package.json Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni
Copy link
Member Author

@magicmatatjahu it's all ready now, switched the library to the official AsyncAPI library.

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

LGTM except this:

Captura de Pantalla 2022-11-20 a las 18 23 17

Please, make this text more readable :)

# Conflicts:
#	package-lock.json
#	src/components/Visualiser/FlowDiagram.tsx
#	src/components/Visualiser/Nodes/ApplicationNode.tsx
#	src/components/Visualiser/Visualiser.tsx
#	src/components/Visualiser/utils/node-factory.ts
@jonaslagoni jonaslagoni requested a review from fmvilas November 21, 2022 19:21
@jonaslagoni
Copy link
Member Author

Done @fmvilas 🙂

@jonaslagoni
Copy link
Member Author

Regarding the code smell it's because of different types that will clash because each library uses different parser versions. Choose to leave it as is, let me know if you want to disable the rule for that line.

This is until the library support the new version: asyncapi/EDAVisualiser#10

fmvilas
fmvilas previously approved these changes Nov 21, 2022
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

🚀

# Conflicts:
#	package-lock.json
#	package.json
#	src/components/Visualiser/Controls.tsx
#	src/components/Visualiser/FlowDiagram.tsx
#	src/components/Visualiser/Nodes/ApplicationNode.tsx
#	src/components/Visualiser/Nodes/PublishNode.tsx
#	src/components/Visualiser/Nodes/SubscribeNode.tsx
#	src/components/Visualiser/Visualiser.tsx
#	src/components/Visualiser/VisualiserTemplate.tsx
#	src/components/Visualiser/utils/node-calculator.ts
#	src/components/Visualiser/utils/node-factory.ts
@magicmatatjahu
Copy link
Member

magicmatatjahu commented Nov 24, 2022

@jonaslagoni When I switch to the visualiser I see this in console and Studio is broken:

image

Which version of react and react-flow the EDAVisualiser uses? Studio now uses latest react (v18) and latest react-flow (v11) version and probably EDA old ones and we have mismatch versions of React.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Mar 29, 2023
@github-actions github-actions bot closed this Jul 28, 2023
KhudaDad414 pushed a commit to KhudaDad414/studio that referenced this pull request Oct 12, 2023
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.

Split out visualizer
4 participants