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: Bookmark Order Adjustment When Moving Up #3634

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

ohneda
Copy link
Contributor

@ohneda ohneda commented Aug 14, 2024

Summary

This PR fixes a minor issue in the bookmark reordering functionality.

Details

Current Behavior

When moving a bookmark upwards (e.g., from position 4 to 2), the current logic incorrectly adjusts the positions of other bookmarks. It increments the position of bookmarks between the new and old positions, leading to conflicts.

Consider the following bookmark structure:

Tag Initial Position
bookmark1 1
bookmark2 2
bookmark3 3
bookmark4 4
bookmark5 5

If we move the bookmark initially at position 4 to position 2, Bookmarks with positions greater than 2 and less than 4, excluding 2 and including 4, are incremented by 1:

Tag Initial Position New Position Change
bookmark1 1 1 -
bookmark4 4 2 manual
bookmark2 2 2 -
bookmark3 3 4 +1
bookmark5 5 5 -

Corrected Behavior

Bookmarks with positions greater than 2 and less than or equal to 4, including 2 and excluding 4, should be incremented by 1:

Tag Initial Position New Position Change
bookmark1 1 1 -
bookmark4 4 2 manual
bookmark2 2 3 +1
bookmark3 3 4 +1
bookmark5 5 5 -

The fix ensures that when moving a bookmark upwards:

  1. The selected bookmark is placed in its new position.
  2. All bookmarks between the new position and the original position (inclusive of the new position, exclusive of the original) are shifted down by one.
  3. Bookmarks outside this range remain unchanged.

This correction maintains the intended order and prevents position conflicts.

Note: The current logic works fine when moving a bookmark from a higher position to a lower position.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

The fix has been manually tested to ensure correct functionality in various reordering scenarios.

Checklist

Please delete any irrelevant options.

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings
  • Local unit tests pass with my changes

@danny-avila danny-avila merged commit e47c3f4 into danny-avila:main Aug 16, 2024
1 check passed
kenshinsamue pushed a commit to intelequia/LibreChat that referenced this pull request Sep 17, 2024
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.

2 participants