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

Display object name plural instead of object #10228

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

bosiraphael
Copy link
Contributor

Display object name plural instead of 'Object' inside the command menu section related to the current object.

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 enhances the command menu by replacing the generic 'Object' label with the plural form of the current object's name, providing more context-aware navigation.

  • Added object metadata state management in /packages/twenty-front/src/modules/command-menu/components/CommandMenu.tsx to fetch and display plural object names
  • Implemented fallback to 'Object' label when metadata is unavailable for graceful degradation
  • Utilized useObjectNamePluralFromSingular hook for consistent object name handling across the application

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

Comment on lines +58 to +65
const objectMetadataItems = useRecoilValue(objectMetadataItemsState);
const currentObjectMetadataId = useRecoilComponentValueV2(
contextStoreCurrentObjectMetadataIdComponentState,
);

const currentObjectMetadataItem = objectMetadataItems.find(
(objectMetadataItem) => objectMetadataItem.id === currentObjectMetadataId,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider memoizing this find operation with useMemo to prevent unnecessary recalculations on re-renders

Copy link
Contributor

@prastoin prastoin left a comment

Choose a reason for hiding this comment

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

Sounds good to me ! Well done

@@ -54,6 +55,15 @@ export const CommandMenu = () => {
'command-menu-previous',
);

const objectMetadataItems = useRecoilValue(objectMetadataItemsState);
const currentObjectMetadataId = useRecoilComponentValueV2(
Copy link
Contributor

@prastoin prastoin Feb 15, 2025

Choose a reason for hiding this comment

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

Nitpick: I was wondering if there were any existing hooks that were already doing that, the best I could find is objectMetadataItemFamilySelector which could be interesting to refactor adding granularity on objectMetadataItem.id which could fit our need here !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The hooks that we normally use to retrieve objectMetadataItem are useObjectMetadataItemById or useObjectMetadataItem. The problem of these hooks is that they throw if objectMetadataItem is undefined. The command menu is one of the few places where the objectMetadataItem can be undefined, because we can open it on a settings page for instance. I wanted to introduce a hook which retrieves the objectMetadataItem but doesn't throw, and I discussed this issue a few time with @lucasbordeau and @charlesBochet.

@bosiraphael bosiraphael merged commit 9ddaaa8 into main Feb 17, 2025
47 checks passed
@bosiraphael bosiraphael deleted the r--display-object-name-plural-instead-of-object branch February 17, 2025 09:35
eliezer-rodrigues037 pushed a commit to mind-developer/kvoip-v2 that referenced this pull request Feb 17, 2025
Display object name plural instead of 'Object' inside the command menu
section related to the current object.
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.

2 participants