-
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 position calculations -- favorites #9202
Fix position calculations -- favorites #9202
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 PR fixes position calculation issues in the favorites system by modifying how positions are handled during drag-and-drop operations and list reordering.
- Changed position calculation in
calculateNewPosition.ts
to subtract 1 instead of dividing by 2 when moving items to start - Updated
useHandleFavoriteDragAndDrop.ts
to start positions at 1 for empty lists and exclude dragged items from position calculations - Modified mock data in
useFavorites.ts
to use consistent position values (1 instead of 0) for folder moves - Fixed edge case in drag-and-drop when moving items between folders by using proper position references
- Added proper position handling for empty destination folders during drag operations
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
if (destinationIndex === 0) { | ||
return items[0].position / 2; | ||
return items[0].position - 1; | ||
} |
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.
logic: Subtracting 1 from position could eventually cause integer underflow with repeated moves to start. Consider using a larger decrement (e.g. -100) or position normalization.
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.
🤔
packages/twenty-front/src/modules/favorites/utils/calculateNewPosition.ts
Show resolved
Hide resolved
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.
LGTM
Thanks @ehconitin for your contribution! |
Co-authored-by: Weiko <[email protected]>
No description provided.