add: related object name to favorite in navigation drawer #8659
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds object type labels to favorites in the navigation drawer, displaying the related object name (e.g. "Company", "Person") next to each favorite item with a bullet point separator.
Key changes:
- Added
objectNameprop toNavigationDrawerItemwith styled container showing "· ObjectName" format - Implemented hover behavior to hide object name when hovering favorite items
- Added
StyledItemObjectNamecomponent with proper styling and text capitalization - Passed
objectNameSingularfrom favorite objects through to navigation components - Fixed display formatting with proper spacing and text overflow handling
4 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
...twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx
Outdated
Show resolved
Hide resolved
| .navigation-drawer-item:hover & { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
logic: Hiding content on hover may cause accessibility issues for keyboard/screen reader users
| </NavigationDrawerAnimatedCollapseWrapper> | ||
| {objectName ? ( | ||
| <StyledItemObjectName> | ||
| {' · '} |
There was a problem hiding this comment.
style: Non-breaking space should be used instead of regular space for better typography
|
@Bonapara this looks cool, could you review the behavior!? |
@harshrajeevsingh could you make sure only favorite's label is being truncated and not all of the navigation drawer item. |
|
@ehconitin Oh! I missed that part. I will make changes soon. |
| {objectName ? ( | ||
| <StyledItemObjectName> | ||
| {' · '} | ||
| {objectName.charAt(0).toUpperCase() + objectName.slice(1)} |
There was a problem hiding this comment.
use capitalize from packages/twenty-front/src/utils/string/capitalize.ts
| <NavigationDrawerAnimatedCollapseWrapper> | ||
| <StyledItemLabel>{label}</StyledItemLabel> | ||
| </NavigationDrawerAnimatedCollapseWrapper> | ||
| <StyledLabelParent> |
There was a problem hiding this comment.
where is the <NavigationDrawerAnimatedCollapseWrapper?
There was a problem hiding this comment.
we need it for collapse animation
There was a problem hiding this comment.
@martmull That's the issue. Using NavigationDrawerAnimatedCollapseWrapper won't let the text truncate for any item because a width constraint is needed for the parent element to make it work.
Also, you can find StyledRightOptionsContainer having margin-left: auto, but it doesn't work due to the NavigationDrawerAnimatedCollapseWrapper, we can see it working due to the StyledSpacer.
I will see how I can make changes to the NavigationDrawerAnimatedCollapseWrapper to make flex property work.
| ${({ hasObjectName }) => | ||
| hasObjectName && | ||
| ` | ||
| max-width: 12ch; |
There was a problem hiding this comment.
think we should avoid exotic css properties like this one
Closes: #8549
Screencast.from.2024-11-22.00-35-00.webm