RocketChat / EmbeddedChat

An easy to use full-stack component (ReactJS) embedding Rocket.Chat into your webapp
https://www.npmjs.com/package/@embeddedchat/react
107 stars 214 forks source link

Refactor sidebar components #509

Closed JeffreytheCoder closed 3 months ago

JeffreytheCoder commented 3 months ago

Brief Title

Acceptance Criteria fulfillment

Fixes #504

Video/Screenshots

All menus look the same as before

JeffreytheCoder commented 3 months ago

Anyone knows why the lint check workflow fails with the following error?

>  Lerna (powered by Nx)   Running target format:check for 6 projects failed

   Tasks not run because their dependencies failed or --nx-bail=true:

   - e2e-react:format:check
   - @embeddedchat/htmlembed:format:check

   Failed tasks:

   - @embeddedchat/react:format:check
Spiral-Memory commented 3 months ago

Anyone knows why the lint check workflow fails with the following error?

>  Lerna (powered by Nx)   Running target format:check for 6 projects failed

   Tasks not run because their dependencies failed or --nx-bail=true:

   - e2e-react:format:check
   - @embeddedchat/htmlembed:format:check

   Failed tasks:

   - @embeddedchat/react:format:check

Format with prettier.

Run it in this file : src/components/Sidebar/Sidebar.module.css

Spiral-Memory commented 3 months ago

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

JeffreytheCoder commented 3 months ago

@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: sidebar-after

I'll create another PR to put pinned & starred messages into the general sidebar after #501 is reviewed.

Spiral-Memory commented 3 months ago

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.

JeffreytheCoder commented 3 months ago

Yes it works :)

many-threads

Spiral-Memory commented 3 months ago

Awesome 🥳🥳

JeffreytheCoder commented 3 months ago

@sidmohanty11 Ready for review

JeffreytheCoder commented 3 months ago

@sidmohanty11 Same as before, the sidebar expand to full screen:

sidebar-mobile