feat: Revamping Sidebar

[JeevaRamanathan]

papermark/components/Sidebar.tsx

Line 156 in c27f3f6

{/* Sidebar component, swap this element with another sidebar if you like */}

Hi @mfts,
Since ShadCN introduced the sidebar component two days ago, I thought of revamping the current sidebar using ShadCN's component. I would like to get your input on this.

Here’s a preview of how it looks:

Mobile View:

[vercel]

@JeevaRamanathan is attempting to deploy a commit to the mftsio Team on Vercel.

A member of the Team first needs to authorize it.

[mfts]

@JeevaRamanathan nice. I was hoping someone would add Shadcn sidebar :)

[JeevaRamanathan]

@mfts I would like to get your input on this.
Do you prefer keeping the sidebar icons visible when it’s closed, or would you rather have a full-screen version like this example from ShadCN: https://ui.shadcn.com/blocks/default/sidebar-05 and kindly let me know if you have any other inputs or suggestions.

[mfts]

@JeevaRamanathan

Do you prefer keeping the sidebar icons visible when it’s closed

Yes it would be great if the icons would still be visible when closed.

[mfts]

@JeevaRamanathan let me know when you want me to start reviewing this PR or have questions :)

[JeevaRamanathan]

Hi @mfts,
Sorry for the delay, was in the middle of something :)

I have removed the commented lines of the previous sidebar along with few changes. I also noticed the visitor option was added, and I have included that as well and resolved the henceforth conflict.
I can say it's ready for review now, but there are two things to note:

1. Currently, the sidebar is not controlled (i.e., when switching pages, it returns to its default open/close state—I will be looking into that).
2. Adding this sidebar creates multiple scroll bars, including one in the AppLayout (here). While adding overflow-hidden removes the extra scroll bars, it makes the items partially hidden at the bottom. Your input on this would be helpful.

Edit:
For 2, I have moved the trigger within the container with a separator which solved this issue. Please have a look and let know on any changes.

23.10.2024_02.51.14_REC.mp4

[JeevaRamanathan]

Hi @mfts,
Finally, the sidebar controlled state issue was resolved with help from this X thread, and it’s now working locally. However, the changes also involve updates in ui/sidebar.tsx. Since a PR might eventually be proposed to ShadCN by the thread author involving changes to ui/sidebar.tsx, I made updates in that file locally as well to get it working based on the thread. So can we address this as a separate issue after this PR merged to avoid any potential clash once the sidebar is updated in ShadCN.

Your thoughts on this please?

[mfts]

@JeevaRamanathan great progress.

I think the collapsible icon should be on the sidebar itself, not on the main div.

the icons and font of the items are one size too small

On mobile, the navbar should contain the collabsible buttons (which is does) and the user dropdown, which it used to but not longer.

Remove the P icon next to Papermark, because there's no space for the plan name

[JeevaRamanathan]

Hi @mfts, I have a question on one section

I think the collapsible icon should be on the sidebar itself, not on the main div.

I felt placing the icon separately from the sidebar might provide more clarity, as the sidebar already contains the title, and combining them could make it feel cluttered. Here’s a similar approach used in ShadCN’s sidebar example, where the trigger is in the main div: Sidebar example and documentation.

the icons and font of the items are one size too small

Got it! I’ll check on this

and the user dropdown, which it used to but not longer.

I’m not sure I completely understood this part. Could you clarify? Are you suggesting that the user dropdown should be removed from the sidebar on mobile view, possibly moving it to the highlighted rounded area on the right side like now?

Remove the P icon next to Papermark, because there's no space for the plan name

Understood! I’ll remove the icon when the sidebar is expanded, and I’ll keep it when it’s collapsed to avoid leaving an empty space.

[JeevaRamanathan]

Hi @mfts, Can you please review the changes

1. Removed the P icon when sidebar is open.
2. Added back the comments in tailwind.config.js
3. Moved the user dropdown to top in mobile view.
4. The font size remains the same as before, but the icon size was smaller than specified. This appears to be a bug in the shadcn sidebar: [bug]: Sidebar icon size is not taken in consideration. Implemented a temporary workaround with custom CSS and made a minor change in ui/sidebar.tsx to fix that.

Could you please take a look and let me know if any changes are needed?

Thanks!

[mfts]

@JeevaRamanathan

I felt placing the icon separately from the sidebar might provide more clarity, as the sidebar already contains the title, and combining them could make it feel cluttered. Here’s a similar approach used in ShadCN’s sidebar example, where the trigger is in the main div: Sidebar example and documentation.

I agree.
However, I don't love how it's currently solved. We are wasting a lot of space on top.

I think the https://ui.shadcn.com/blocks#sidebar-07 should be our role model

• Let's keep Papermark at the top.
• move the plan into the dropdown team picker.
• maybe we can even use a nicer dropdown or whatever shadcn block uses here

I also like these collapsibles to reveal more settings

[mfts]

I'm awarding you now but I'd love for you to continue with this sidebar so we can get it merged :)

/award 750

[oss-gg]

Awarding JeevaRamanathan: 750 points 🕹️ Well done! Check out your new contribution on oss.gg/JeevaRamanathan

[JeevaRamanathan]

Hi @mfts,

• Let's keep Papermark at the top.
• move the plan into the dropdown team picker.
• maybe we can even use a nicer dropdown or whatever shadcn block uses here

I have made these changes similar to sidebar-07.

I also like these collapsibles to reveal more settings

We already have branding as a separate option within the settings screen. Should we move the branding section into the collapsible settings as well to reveal more options?