style(website): make Playground Mobile Responsive #2028
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
✅ Deploy Preview for modelina canceled.
|
jerensl
changed the title
|
The new commit should fix the current issue, I'm adding extra state for updating the costume theme |
There was a problem hiding this comment.
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.
| renderModels, | ||
| setRenderModels, | ||
| ]); | ||
| const contextValue = useMemo( |
There was a problem hiding this comment.
Please don't make unnecessary formatting changes, revert these changes.
There was a problem hiding this comment.
It's been part of the eslint-config-prettier rule and done automatically by prettier from the parent eslint rules, can we have npm run lint:fix as we did in the parent directory?
There was a problem hiding this comment.
Okay sure, can you please create a separate PR for adding the npm run lint:fix command.
To keep the desktop version, would you mind if I changed the layout approach from The bug just happened after we added conditional rendering for supporting mobile version, so it related to this feature |
…ack desktop design
|
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 #2028 (comment) |
Okay sure, if it doesn't break anything. |
Pull Request Test Coverage Report for Build 9438059815Details
💛 - Coveralls |
There was a problem hiding this comment.
Things look good to me, but we need to have proper lint fix rules before merging this PR. Otherwise, it's creating many formatting changes.
@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?
|
@jonaslagoni Can you also check the playground layout on mobile and desktop and share your thoughts on it? |
Makes sense to me 👍 Just remember to add the website to the root ignore files so there wont be any duplicates. |
|
Noted, I already made a changes and added the final detail in the latest commit and editted the detail changes I made above |
jerensl
changed the title
jerensl
changed the title
jerensl
changed the title
Pull Request Test Coverage Report for Build 9561440615Details
💛 - Coveralls |
There was a problem hiding this comment.
@jonaslagoni, I am waiting for the PR #2031 to be merged first before reviewing and merging this one. As there are many changes just made by prettier configuration. Or should we merge this one for now?
There was a problem hiding this comment.
Looks good to me. The formatting issue will be handled in #2031
Pull Request Test Coverage Report for Build 9626631986Details
💛 - Coveralls |
|
/rtm |
|
🎉 This PR is included in version 3.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Enable Playground work on Mobile Devices
FlextoGridwhich reduces multiple unnecessary div components and enables us to keep the existing desktop designMobile
Tablet
Notebook
Desktop
Alternative Solution
GridRelated Issue
This refactor relates to #1715
Checklist
FlextoGridwhich reduces unnecessary HTML components and makes the website more responsiveGridAdditional 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