Fix: Replace styled logo with Avatar component for workspace logo in NavigationDrawerHeader #9093
Conversation
Changed the padding-left for the chip component when the variant is Transparent. The padding-left is now set to 2px to ensure proper alignment, while other variants continue to use the default horizontal padding.
- Increased padding-left for table cells from 6px to 8px for uniform spacing. - Set padding to 0px for Chip component in transparent mode to prevent it from affecting cell padding and ensure consistent 8px left padding across all cells.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Replaced the styled logo with Avatar component in NavigationDrawerHeader to provide consistent workspace logo display with fallback placeholder functionality.
- Implemented Avatar component in
/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerHeader.tsxusing workspace name's first character as placeholder - Removed StyledLogo component and isNonEmptyString check for cleaner implementation
- Added proper fallback handling when logo URL is invalid or undefined
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
Thanks @muraliSingh7. While testing your PR, I figured out that the multi-workspace case was not taken into consideration. I have replaced by Avatar component in multiworkspace dropdown too
| <StyledLogo | ||
| logo={isNonEmptyString(logo) ? logo : DEFAULT_WORKSPACE_LOGO} | ||
| /> | ||
| <Avatar placeholder={name} avatarUrl={logo} /> |
There was a problem hiding this comment.
no need to do name.charAt(0) here as the Avatar is already doing it internally
| @@ -16,7 +17,7 @@ export const PageFavicon = () => { | |||
| getImageAbsoluteURI({ | |||
| imageUrl: workspacePublicData.logo, | |||
| baseUrl: REACT_APP_SERVER_BASE_URL, | |||
| }) ?? '' | |||
| }) ?? DEFAULT_WORKSPACE_LOGO | |||
There was a problem hiding this comment.
@Bonapara FYI, I'm also making sure we have a Favicon (we cannot use the first letter trick here as the logo needs to be a png)
|
Thanks @muraliSingh7 for your contribution! |
- Fixed icon shrinking on tabs. The introduction of EllipsisDisplay takes 100% of the width, thus shrinking the icons. - Removed scroll wrapper tablist. This was removed in #9016 but reintroduced in #9089. This reintroduction made the dark border below the active tab disappear. - Used Avatar for icon and logo rendering following the changes made in #9093
This PR addresses issue #9042
Avatarcomponent.Avatarnow handles the case when nologois uploaded by using the workspace name's first character as theplaceholder.logois undefined.