Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
| ? [...items, newItem] | ||
| : toSpliced(items, itemToEditIndex, 1, newItem); | ||
|
|
||
| onChange(updatedItems); |
There was a problem hiding this comment.
Bug: The handleClickOutside function in MultiItemFieldInput passes stale items to its callback, ignoring recent user modifications before the click.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
In the MultiItemFieldInput component, the handleClickOutside function passes stale data to its callback. When a user modifies an item and clicks outside the input, the validateInputAndComputeUpdatedItems function computes the new state of the items. However, the onClickOutside callback is then invoked with the original items prop, not the newly computed updatedItems. This results in the parent component receiving outdated data, leading to data loss or UI inconsistency as the user's changes are not correctly persisted.
💡 Suggested Fix
Capture the updatedItems returned from validateInputAndComputeUpdatedItems(). Then, in handleClickOutside, pass this updatedItems variable to the onClickOutside callback instead of the original items prop to ensure the parent component receives the most current data.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/MultiItemFieldInput.tsx#L220-L223
Potential issue: In the `MultiItemFieldInput` component, the `handleClickOutside`
function passes stale data to its callback. When a user modifies an item and clicks
outside the input, the `validateInputAndComputeUpdatedItems` function computes the new
state of the items. However, the `onClickOutside` callback is then invoked with the
original `items` prop, not the newly computed `updatedItems`. This results in the parent
component receiving outdated data, leading to data loss or UI inconsistency as the
user's changes are not correctly persisted.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7646049
|
Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Follow up on #16603