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(website): create new playground layout #1628

Merged
merged 33 commits into from
Dec 11, 2023

Conversation

devilkiller-ag
Copy link
Member

Description
This is a draft PR for building a new layout of Modelina Playground. The new layout will be similar to the layout of Async API Studio. The first step will be to create a new layout and then I will be working on the logic. I have created a temporary component file NewPlayground.tsx to build the new layout. We will replace the old components with the new ones once the layout is ready.

Related issue(s)

Resolves #1346

Copy link

netlify bot commented Nov 22, 2023

Deploy Preview for modelina ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/modelina/deploys/65768b6b3fed5a1f04de7bc2
😎 Deploy Preview https://deploy-preview-1628--modelina.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 configuration.

@devilkiller-ag devilkiller-ag marked this pull request as draft November 22, 2023 13:37
Copy link
Contributor

@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.

@devilkiller-ag
Copy link
Member Author

Hi @jonaslagoni @codingmickey, I was trying to use React Split Pane for the section which will show input, output, and the output navigation. But it seems that it haven't been updated since last three years and doesn't support React v18.

I found this new react-split-pane implementation by Jeremy Grieshop which supports React v17 and above. It's also been used in the AsyncAPI Studio. Can I use this instead? The problem is that it isn't available as package so I will have to copy paste whole repo as a component as also done in the studio.

@jonaslagoni
Copy link
Member

@devilkiller-ag I would say no, dont use a tool that has no users 🙂 Then its better to have no split pane functionality.

@devilkiller-ag
Copy link
Member Author

Hi @jonaslagoni, I agree with you. For now I have used the code for the split pane from Async API Studio Codebase. But if you want I can try to implement it from scratch for our playground. What's your opinion?

@jonaslagoni
Copy link
Member

Hi @jonaslagoni, I agree with you. For now I have used the code for the split pane from Async API Studio Codebase. But if you want I can try to implement it from scratch for our playground. What's your opinion?

You can select the direction you want to take, there is no requirement here 🙂 We don't have split panel right now anyway, so it can be added later if desired 👌 Then all you need to focus on is the basic layout, making it easier to achieve.

@devilkiller-ag
Copy link
Member Author

Hi @jonaslagoni, I agree with you. For now I have used the code for the split pane from Async API Studio Codebase. But if you want I can try to implement it from scratch for our playground. What's your opinion?

You can select the direction you want to take, there is no requirement here 🙂 We don't have split panel right now anyway, so it can be added later if desired 👌 Then all you need to focus on is the basic layout, making it easier to achieve.

Okay Sure, I will go with the basic layout for now 🙂

@devilkiller-ag
Copy link
Member Author

I have added the mobile responsive layout without split-pane. Now I will add the Editor, Options, and Output UI.

@devilkiller-ag
Copy link
Member Author

@jonaslagoni The work is complete! Kindly check it out.

@devilkiller-ag devilkiller-ag marked this pull request as ready for review December 9, 2023 02:32
@coveralls
Copy link

coveralls commented Dec 9, 2023

Pull Request Test Coverage Report for Build 7161607198

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.516%

Totals Coverage Status
Change from base Build 7154077028: 0.0%
Covered Lines: 5600
Relevant Lines: 5901

💛 - Coveralls

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Looking great! Got some minor comments 🔥

Here is what I see and what I base the comments on:

  • All the views need to be aligned in height
  • The ? symbol would look better if it blends better with the black background
  • I would add more margins to the menu item, as they are too close to the panel to the right.
  • The view with the generator code and generated models, it's hard to see that generator code can be clicked
  • We should make it more easier to see which output model is being selected.
  • In the generator options view, for example if you select kotlin the panel gets scrollable 🤔 Is there a better way to show long options?

image

@devilkiller-ag
Copy link
Member Author

Hi @jonaslagoni,

I have incorporated all the suggestions. Kindly check it out.

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

You are fast @devilkiller-ag, only got very few remarks. I think this is the final changes needed, checked all the code and it looks great, only got those small points 💪 Cant wait to merge this 🔥

Think we can change the width of the sidebar menu to 50:
image

@jonaslagoni
Copy link
Member

Once we merge this I would love to offer you code ownership of the modelina-website code, let me know if you want to stick around for longer than a single PR 😉

Let me know if you have any questions about it 👍

@jonaslagoni jonaslagoni changed the title feat: create new playground layout chore(website): create new playground layout Dec 11, 2023
@devilkiller-ag
Copy link
Member Author

devilkiller-ag commented Dec 11, 2023

Hi @jonaslagoni, I have made all the changes you suggested, additionally, I have changed the Tooltip color and padding to make it visible. Thanks for offering me the code ownership of the modelina-website. I would love to be able to contribute forward. I had an issue in mind that I was going to open just after this PR closes: Currently, the website code uses class components, we should update the code base to use functional components and hooks at all places. This will result in cleaner, easier-to-understand components compared to class components. For the Playground, I have updated most of the class components to functional components on which I worked. What's your view on this?

Copy link

sonarcloud bot commented Dec 11, 2023

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 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni
Copy link
Member

Hi @jonaslagoni, I have made all the changes you suggested, additionally, I have changed the Tooltip color and padding to make it visible

Perfect 👍

I had an issue in mind that I was going to open just after this PR closes: Currently, the website code uses class components, we should update the code base to use functional components and hooks at all places. This will result in cleaner, easier-to-understand components compared to class components. For the Playground, I have updated most of the class components to functional components on which I worked. What's your view on this?

Sounds good, feel free to create an issue for it to track it 🙂

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

🔥

@jonaslagoni
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit f173cf7 into asyncapi:master Dec 11, 2023
17 checks passed
@jonaslagoni
Copy link
Member

@all-contributors please add @devilkiller-ag for code, design

Copy link
Contributor

@jonaslagoni

I've put up a pull request to add @devilkiller-ag! 🎉

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.0.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.0.0-next.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Improve layout of playground
4 participants