Opening sidebar should not displace main content

[Amnish04]

@humphd

I spent a while to figure out a solution that requires least amount of changes to the existing structure.

I have made the following changes:

1. Grid Columns do not change their width on toggling sidebar.
2. The sidebar is wrapped inside a Box element provided by Chakra, that opens over the main content instead of pushing it away, or squishing it in case of a mobile screen.
3. The width of sidebar is almost similar to how it was initially across various screen widths.
4. The sidebar now has a custom animation when opened and closed (enhancing user experience).
5. Nothing else was changed in the original layout.

Please let me know if any changes are required.

Fixes #299

[sweep-ai]

Apply Sweep Rules to your PR?

• Apply: All new business logic should have corresponding unit tests.
• Apply: Refactor large functions to be more modular.
• Apply: Add docstrings to all functions and file headers.

[humphd]

Can you please rebase this on main so it picks up the database migration stuff that landed recently?

[Amnish04]

Just rebased and pushed the squashed commit.

[humphd]

Some general comments:

• The animation fires onload even when the sidebar won't show. I think you'd probably never want to show this onload
• There's a weird breakpoint issue that makes it really narrow. Probably it should have a larger min-width:

• If we want this to be a temporary UI that slides out, you do something, it slides in again, maybe we should use the built-in .

@tarasglek what's your opinion of this? Do you care if the sidebar is persistent? Overlaps content vs. sits beside it?

[Amnish04]

• Just fixed the unnecessary close animation which was triggered whenever isSideVisible was false.
• Also set a min-width of 300px on the sidebar so it sizes like in original layout across different widths.
• I tried using the in-built Slide transition in my first attempt, but the problem is it spans accross the full viewport-height. And when I try to position it absolutely with respect to the parent GridItem, the Slide animation stops working.

[Amnish04]

@humphd If this doesn't work well, we can only apply this behavior on smaller breakpoints where content gets squished. I noticed that ChatGPT also does the same way.
I can push the changes if you agree.

[humphd]

I showed this to some users, and a few want to know if it can be made so you can pin it open. By default it can close, but some people want the side bar open all the time. Should be a pref with UI to allow changing it.

[Amnish04]

I've made the following changes based on user feedback:

• The sidebar can now be pinned on Desktop screens, as mobile screens do not have enough real estate.

• Added a new setting called sidebarPinned to the Settings model.
  

• Added a useDesktopBreakpoint hook similar to the mobile version to only query proper desktop width (the pinned sidebar cannot be supported below a certain width, which is why used desktop first approach).

• The toggle button has appropriate title and aria-label (although I guess we don't need aria-label when there's a title)

When Sidebar is Pinned:

Normal and Default Behavior:

[humphd]

I'm confused. I pulled this locally and tried it but the animation still plays on load, there's no way to pin it. Am I looking at the wrong thing somehow?

I'm on faa2c87

[Amnish04]

@humphd
Weird, I just rechecked locally

seems to work fine for me.
Can you share a screenshot of what you see?

Edit: Also, checked in Github Codespaces

What I see:

The only scenario when the animation plays on load is when the sidebar is meant to be opened, which was more of a feature from my perspective. I made sure that there is no redundant close animation when the sidebar is meant to be closed on load.
Maybe you are referring to that?

[humphd]

OK, I completely missed that new icon, which is a bad sign. I think we need to do some more work on this.

Here's an example of how a lot of drawers, which support open/close states, work: https://sparkmailapp.com/help/tips-tricks/new-expanded-sidebar. Here are some suggestions for next steps here:

1. Any icons that are related to this drawer needs to be on the left-hand side, so it's clear what it goes with
2. We should probably drop the hamburger menu and replace it with a few sidebar icons, which better reflect the state of the drawer (open vs. closed)

Another example: GitHub's file tree (closed vs. open)

I also don't think we should animate it when hitting the breakpoint as the window expands. If it fits all of a sudden, and you want to show it, do that, but don't draw attention to it.

[Amnish04]

Just made some changes based on the feedback.

1. I replaced the hamburger icon with sidebar icons as suggested.

Sidebar Collpapsed:

Sidebar Explanded:

2. I tried to relocate the pin sidebar icon so users can easily find it.

To make the purpose of the button clearer, I have replaced the native title property with Tooltip from Chakra.

I am not sure if that's the best place to put it, if you can think of a better location please let me know.

3. For sidebar open animation, it is an edge case that only occurs when you pin the sidebar open on larger screen, close it on the smaller screen and then try to resize to a larger window again.
The reason is that the sidebar open and close state is managed with the animation property, and I cannot think of a way to determine that isSidebarVisible || (settings.sidebarPinned && isDesktop) expression became true due to resizing of window.

In any case, this is a pretty rare occurence and shouldn't bother users much as a user would typically work on the same screen size. Please let me know what you think about this.

[kliu57]

@Amnish04 I tested the functionalities in browser. Have not looked at the code.

The functionalities work as you described (side bar now overlaps the main program rather than pushing it to the right; sidebar can be pinned)

However I am not sure if this will benefit users that are using a narrower browser screen, since they won't be able to type to ChatCraft unless they minimize the sidebar, whereas previously they were able to. (see screenshot showing production behaviour and behaviour of this PR). Require input from admins on this @humphd @tarasglek

Behaviour in Narrower Chrome window or iPad Mini :

[Amnish04]

@Amnish04 I tested the functionalities in browser. Have not looked at the code.

The functionalities work as you described (side bar now overlaps the main program rather than pushing it to the right; sidebar can be pinned)

However I am not sure if this will benefit users that are using a narrower browser screen, since they won't be able to type to ChatCraft unless they minimize the sidebar, whereas previously they were able to. (see screenshot showing production behaviour and behaviour of this PR). Require input from admins on this @humphd @tarasglek

Behaviour in Narrower Chrome window or iPad Mini :

The problem with Mobile View is that everything gets squished to a very small width, which is not a very good UX.
Current:

Overlap:

ChatGPT also handes this in the same way.

[kliu57]

@Amnish04 I know that chatcraft production and chatgpt all don't handle very small screen sizes well (less than width 774px approx~) when the sidebar is out, but for browser width such as 774px (this is an approximation) or more chatcraft prod and chatgpt is still useable with the sidebar out.

Chatgpt:

[Amnish04]

@kliu57 I was also thinking about that. Maybe we should just do the overlap behaviour for mobile screens. The desktop behaviour can stay the same, except the pin sidebar feature would be an addition.

[Amnish04]

Continued in #437