-
Notifications
You must be signed in to change notification settings - Fork 262
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 sidebar components #509
Refactor sidebar components #509
Conversation
Anyone knows why the lint check workflow fails with the following error?
|
Format with prettier. Run it in this file : src/components/Sidebar/Sidebar.module.css |
@JeffreytheCoder , This is an awesome change, great work bro!. It was difficult to achieve as well. Well, I suggest attaching a video showcasing everything working smoothly (all sidebar components) without any rendering issue or console errors for every functionality in those sidebars. I can see that you have only extracted the very common elements and passed the other part as a children prop. While it's not extremely modular, which is good as it reduces the chances of introducing bugs, as discussed earlier there is still potential for issues and since we have a plan to release it as next major version soon. Therefore, please ensure from your side as well, and including a video with no console errors will be helpful. |
@Spiral-Memory Thanks bro! It's hard to modularize sidebar content because they vary a lot, but having a modularized sidebar wrapper solves some inconsistent stylings, and it's better to maintain and add upcoming features like resizing along with the chat window. Here's the recording below that shows all sidebars work as before, with no console warning or error: I'll create another PR to put pinned & starred messages into the general sidebar after #501 is reviewed. |
Yep, I told you, it has so much variation and is hard to modularize, hehe! Anyway, great work! Yes, and until then, the file menu feature will also be added. Refactor both together. Is the thread scrollbar working fine? Create many threads and check. |
Awesome 🥳🥳 |
@sidmohanty11 Ready for review |
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.
@JeffreytheCoder awesome! Had one question, how does it look in minimised mode?
@sidmohanty11 Same as before, the sidebar expand to full screen: |
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.
Thanks @JeffreytheCoder !!
Brief Title
Acceptance Criteria fulfillment
Fixes #504
Video/Screenshots
All menus look the same as before