-
Notifications
You must be signed in to change notification settings - Fork 82
refactor: split <WorkspacePanel>
into 3 internal components
#253
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
refactor: split <WorkspacePanel>
into 3 internal components
#253
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not! 🤩
Given that part is probably less likely to change at this point, this is a nice change!
I would use different name than Terminal
, Editor
and Preview
as it would make reading this code harder IMO as we already have components named like this.
34c8b98
to
4ff2971
Compare
Weird that GitHub closes PRs when a PR reference a comment from another PR 😅 |
Alright I think this is ready now! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good! I love the improvements, the code is a lot better! 🤌
Just a minor nit and let's get this merged 🥳
<WorkspacePanel>
internally<WorkspacePanel>
is currently difficult as it contains so much logic. Everything is declared inside a single React component. Let's split this component into three smaller ones to improve readability and maintainability.Tested manually but let's instead wait for #241 just to be sure.
@Nemikolh any thoughts about this?