Align Workspace Switcher with Breadcrumb by Adjusting Height

[ehconitin]

@Bonapara
Issue #6375

This change makes sure the container height is 32px instead of 28px.
should the container inside it should also be 32px, please refer video below for context

2024-07-24.00-38-44.mp4

Also skeleton height is 20px (refer video below), the black component in the video is the skeleton for this particular component.
What should be skeletons height?

2024-07-24.00-55-29.mp4

[greptile-apps]

PR Summary

Adjusted the height of the StyledContainer in NavigationDrawerHeader.tsx to align the workspace switcher with the breadcrumb.

• Modified packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerHeader.tsx: Adjusted StyledContainer height from 28px to 32px.
• Ensure the height change does not affect other dependent components or layouts.
• Verify skeleton alignment and height consistency across the UI components.

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings}

[Bonapara]

@ehconitin, it's okay that the skeleton height is 20px (the height of the icon/text), but it should be vertically centered within the container. Thanks!

[ehconitin]

@Bonapara

This might sound absurd but when I am checking skeleton now its of height 16px which is also the size of logo and text
but the allignment is completely off w.r.t the adjacent skeleton. But you could see in yesterdays PR description the video does show the height as 20px.
I am baffled

Refer the video.
Is this the same for you ?
A bit confused :(

2024-07-24.19-15-16.mp4

[Bonapara]

Hi @ehconitin, here is the figma of the skeleton loading. Maybe you can check the measurements on Figma? Tell me if it's still not clear

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=25426-60098&t=ISpVG0j2SLv3paOI-11

[ehconitin]

Ill have a look at it @Bonapara, if I still cant figure it out will reach out to you.

[ehconitin]

@Bonapara
I hardcoded styles from figma.
please let me know what do you think.
Is this approach good??

skeleton base color is rendered as black for better visibility

[Bonapara]

Hi @ehconitin, what do you mean by hardcoded? In Figma, I used the same component what about in the code?

[ehconitin]

@Bonapara,
In code the skeleton and workspace switcher do not share same component. If that was the case it would be very easy and smooth fix. Thats a good point, I think they should be sharing the same component which will make future changes or implementation way easier. For now its a balancing act on how do I match skeletons styles to that of Workspace switcher (or vice versa). Its the same with RightPanelSkeletonLoader
@lucasbordeau what do you think?

[charlesBochet]

Hi @ehconitin, thanks for the detailed approach.

Agree, right now we have a big component containing nav skeleton which needs to be updated when we touch the nav which will likely lead to an outdated skeleton. What would be great is

• to have a React context "NavSkeletonLoaderContext" containing the information if the skeleton should be displayed or not (this way we don't have to compute it in multiple places like what we have right now)
• but then break the big skeleton into smaller ones loaded by each part of the Navbar and reading from this context

I think this can be done in another PR, would be great if you can tackle it!

[charlesBochet]

LGTM, thank you!

[github-actions]

Thanks @ehconitin for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

[ehconitin]

@charlesBochet
Thanks,
Could u create an issue for me to work on the navskeletoncontext.
Would love to work on it.