Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add object names to favorites #8593

Closed
wants to merge 3 commits into from
Closed

Add object names to favorites #8593

wants to merge 3 commits into from

Conversation

BKM14
Copy link
Contributor

@BKM14 BKM14 commented Nov 19, 2024

Closes #8549
The favorites tab now also displays the object names.

There seems to be a text overflow error with the code as it is. It is not truncating automatically leading to a "clip" behavior instead of an "ellipsis". I tried fixing that, but could not. The Object name should get truncated. Hope someone can guide me on this

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR adds object names to favorites in the navigation drawer, with a text overflow issue where long object names are being clipped instead of showing ellipsis.

  • Added StyledItemLabelWithObjectName in NavigationDrawerItem.tsx but text-overflow styling not working correctly
  • Added isFavorite and objectName props to both NavigationDrawerItem and NavigationDrawerSubItem components
  • Implemented CreateLabelWithObjectName utility for proper object name formatting with capitalization
  • Added separator dots between favorite label and object name with proper spacing

4 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +135 to +140
const StyledItemLabelWithObjectName = styled.span`
color: ${({ theme }) => theme.font.color.light};
font-weight: ${({ theme }) => theme.font.weight.regular};
text-overflow: ellipsis;
white-space: nowrap;
`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Missing overflow property and max-width constraint needed for text-overflow: ellipsis to work. Add overflow: hidden and max-width.

Comment on lines 147 to 149
favorite.labelIdentifier +
' . ' +
String(favorite.objectNameSingular?.charAt(0).toUpperCase()) +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Inconsistent separator - using ' . ' (space-dot-space) here but ' · ' (space-middot-space) in the component render. Should use the same separator consistently.

@@ -36,6 +37,8 @@ export const NavigationDrawerSubItem = ({
keyboard={keyboard}
rightOptions={rightOptions}
isDraggable={isDraggable}
isFavorite
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Setting isFavorite to true by default may be too restrictive - consider making this configurable via props like the other properties

@BKM14
Copy link
Contributor Author

BKM14 commented Nov 22, 2024

Hey @Bonapara, can you review this please?

@FelixMalfait
Copy link
Member

@BKM14 I'm sorry you were the first to raise a PR on this issue so we should have reviewed this PR earlier but we reviewed the second PR. Since it seems to be more advanced we'll close yours. Thanks a lot for your effort. Hope you can find what's missing by looking at the other one: #8659

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add related object name to the favorite in navigation drawer
2 participants