-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: Set field type icon as the default icon for new fields (#7352) #7579
fix: Set field type icon as the default icon for new fields (#7352) #7579
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request improves code organization by moving the default icon mapping for field types to a separate constants file.
- Created new file
packages/twenty-front/src/pages/settings/data-model/constants/FieldTypeIcons.ts
to storedefaultIconsByFieldType
mapping - Updated
SettingsObjectNewFieldConfigure.tsx
to import and use the new constant file - Modified form configuration in
SettingsObjectNewFieldConfigure.tsx
to use the imported mapping for default icon values - Improved maintainability by centralizing the icon mapping in a dedicated constants file
2 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
type: formConfig.watch('type'), | ||
}; | ||
useEffect(() => { | ||
formConfig.setValue('icon', defaultIconsByFieldType[fieldType] || 'IconUsers'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: This useEffect might cause unnecessary re-renders. Consider memoizing the default icon
fieldMetadataItem={fieldMetadataItem} | ||
fieldMetadataItem={{ | ||
icon: formConfig.watch('icon'), | ||
label: formConfig.watch('label') || 'New Field', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using a constant for the default label 'New Field'
@@ -0,0 +1,27 @@ | |||
import { FieldMetadataType } from '~/generated-metadata/graphql'; | |||
|
|||
export const defaultIconsByFieldType: Record<FieldMetadataType, string> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding a brief comment explaining the purpose of this constant
@dostavic, can you use the same icons as in the previous step? For instance the |
Closes #7352
Summary
Moved the
defaultIconsByFieldType
mapping from theSettingsObjectNewFieldConfigure
component to a separate constants file. This change improves code organization and maintainability without changing functionality.Changes Made
FieldTypeIcons.ts
, located insrc/pages/settings/data-model/constants/
, to store the mapping ofFieldMetadataType
to default icons.Updated the import in the component: In the file
SettingsObjectNewFieldConfigure.tsx
, imported the mapping from the new constants file.Adjusted form configuration: Modified
defaultValues
inuseForm
anduseEffect
to use the imported mapping.