jerensl commented 5 months ago

Enable Playground work on Mobile Devices

User be able to choose between input editor or output editor
When opening general options or generator options it will close others, work back and forth
On Mobile devices when opening the general option it will take the full display area, and the active button will indicate it's open
Refactor the code from using Flexto Grid which reduces multiple unnecessary div components and enables us to keep the existing desktop design
Change the first implementation from conditional rendering to using CSS hidden property, so the bug we encounter not become relevant anymore and we remove the quick fix we implemented before
I decided to add more space on the input editor and take it from output result in bigger device notebook/desktop because of user probably will work more on the the input edditor rather then the ouput result

Mobile

Mobile Device

Tablet

Tablet Device

Notebook

Desktop

Alternative Solution

Visible property, The problem with this one is, the element is not being removed form the dom so it will make it difficult to allocate space with Grid

Related Issue

This refactor relates to #1715

Checklist

[x] Make Playground Mobile Responsive .
[x] Fixes Dark Theme UI Bug.
[x] Refactor layout from using Flex to Grid which reduces unnecessary HTML components and makes the website more responsive
[x] Rollback the desktop design and keep it by using Grid

Additional Notes

Be careful using Conditional rendering with monaco-react when the costume theme is not being rendered by the first update, probably related to this issue, the way to solve it is by adding extra state for theme after adding the theme using monacoInstance

netlify[bot] commented 5 months ago

Deploy Preview for modelina canceled.

Name	Link
Latest commit	29975d56ed22bb279014cf90ba77b15c21bb66b6
Latest deploy log	https://app.netlify.com/sites/modelina/deploys/6676f352d903000008638286

jerensl commented 5 months ago

The new commit should fix the current issue, I'm adding extra state for updating the costume theme

jerensl commented 5 months ago

Hi @jerensl, The mobile version looks great to me. However, we don't need any changes to the desktop design. So please revert those changes. Also, could you open a new Issue + PR for bugs unrelated to this issue? Don't merge those fixes in this PR.

To keep the desktop version, would you mind if I changed the layout approach from Flexbox to Grid?

The bug just happened after we added conditional rendering for supporting mobile version, so it related to this feature

jerensl commented 5 months ago

Hi @devilkiller-ag I already removed the quick bug fixes because it's not relevant anymore, I tried a new approach that uses the hidden property, for more detail, you can read my edited comment above https://github.com/asyncapi/modelina/pull/2028#issue-2340750999

devilkiller-ag commented 5 months ago

To keep the desktop version, would you mind if I changed the layout approach from Flexbox to Grid?

The bug just happened after we added conditional rendering for supporting mobile version, so it related to this feature

Okay sure, if it doesn't break anything.

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9438059815

Details

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.303%

Totals
Change from base Build 9382165682:	0.0%
Covered Lines:	5996
Relevant Lines:	6327

💛 - Coveralls

devilkiller-ag commented 5 months ago

@jonaslagoni Can you also check the playground layout on mobile and desktop and share your thoughts on it?

jonaslagoni commented 5 months ago

@jonaslagoni I propose to have same eslint and prettier rules as we have in the asyncapi website. Those rules have been properly brainstormed and implemented recently while migrating it to Next.js v14 and Typescript. Having same lint rules will ensure similar formatting rules across all website codebases in the asyncapi ecosystem. What's your thought on this?

Makes sense to me 👍 Just remember to add the website to the root ignore files so there wont be any duplicates.