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

Fix array edit option #8697

Merged
merged 1 commit into from
Nov 23, 2024
Merged

Fix array edit option #8697

merged 1 commit into from
Nov 23, 2024

Conversation

BKM14
Copy link
Contributor

@BKM14 BKM14 commented Nov 23, 2024

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 an Array type.
It has been added now with an explicit break statement.

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

Added support for editing array type fields in the MultiItemFieldInput component by implementing a case handler for FieldMetadataType.Array in the edit functionality.

  • Added case FieldMetadataType.Array in handleEditButtonClick function within /packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/MultiItemFieldInput.tsx to handle array item editing
  • Implemented array item editing to treat items as strings, similar to email field handling
  • Fixed issue cannot edit array item #8578 where clicking edit on array items would throw an error

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

Comment on lines +120 to +123
case FieldMetadataType.Array:
item = items[index] as string;
setInputValue(item);
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The Array case implementation is identical to Emails case. Consider extracting this common logic to reduce duplication.

@BKM14
Copy link
Contributor Author

BKM14 commented Nov 23, 2024

Hey @ehconitin, I understood what you said about needing an explicit statement for the Array block. I thought it would be repetitive to write the same code again. But what you said makes sense and I've made the changes requested. I created a fresh PR with the changes specified.
Kindly review this, thank you

Copy link
Contributor

@ehconitin ehconitin left a comment

Choose a reason for hiding this comment

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

LGTM!

@FelixMalfait
Copy link
Member

Thank you!

@FelixMalfait FelixMalfait merged commit fd8e0d0 into twentyhq:main Nov 23, 2024
17 checks passed
Copy link

Thanks @BKM14 for your contribution!
This marks your 4th PR on the repo. You're top 8% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

@BKM14 BKM14 deleted the fixArrayEdit branch November 23, 2024 12:38
Copy link

sentry-io bot commented Nov 24, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: Unsupported field type: ARRAY /object/quote/8393b04d-1754-4d1d-bb87-f2259e90d298 View Issue

Did you find this useful? React with a 👍 or 👎

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.

cannot edit array item
3 participants