-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 array edit option #8600
base: main
Are you sure you want to change the base?
Fix array edit option #8600
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
Added support for editing Array type fields in the MultiItemFieldInput component, fixing an error that occurred when attempting to edit Array type values.
- Added
FieldMetadataType.Array
case inhandleEditButtonClick
method in/packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/MultiItemFieldInput.tsx
- Array type values are now handled similarly to Email types, treating them as strings
- Consider adding type safety checks when casting Array values to strings
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
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.
Left a comment
@@ -110,6 +110,7 @@ export const MultiItemFieldInput = <T,>({ | |||
item = items[index] as PhoneRecord; | |||
setInputValue(item.countryCode + item.number); | |||
break; | |||
case FieldMetadataType.Array: |
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.
Why is there no break condition for this? is the break condition for emails and arrays the same?
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.
Yes, the break statement for Array and Email are the same. The way the Array is being handled in the backend is just like the Email. So I found it better to use the same block as the Email one
Hey @ehconitin, could you review this please? |
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.
This works, but I think we should have an explicit break for FieldMetadataType.Array. Having an explicit break makes it immediately clear that this is intentional behavior, not an oversight.
There is an explicit break for the Array though. If it applies for Email, won't it also apply for Array |
Closes #8578
There was no case to handle a
FieldMetadataType.Array
and thus an error was thrown every time the edit button was clicked on anArray
type.It has been added now.