charmbracelet / bubbles

TUI components for Bubble Tea 🫧
MIT License
5.55k stars 265 forks source link

Inconsistent & potentially misleading behaviour when calling List `RemoveItem` #632

Open g026r opened 3 weeks ago

g026r commented 3 weeks ago

Describe the bug

RemoveItem, when called, operates both on the unfiltered list & the filtered list resulting in misleading & inconsistent behaviour.

If index < len(filteredItems) then filteredItems will be updated to remove the item at that index while simultaneously removing the item at the same index in the global list even if the two items do not match.

If index >= len(filteredItems) then the global list will be updated to remove the item at that index while the filtered list is simultaneously not updated, even if the item that was removed is currently visible in the filtered list.

Setup Please complete the following information along with version numbers, if applicable.

To Reproduce Steps to reproduce the behavior:

  1. Have a list.
  2. Filter such that the second item in the global list is the first item in the filtered list
  3. Call RemoveItem(0) on the filtered list
  4. Observe that the filtered list updates to show that the first item in it is removed
  5. Reset the filter
  6. The item removed from the filter is present, but item in position 0 is gone

Alternatively:

  1. Have a list.
  2. Filter such that the global index of the first item in the filtered list is beyond the length of the filter (e.g. filter is len(1) but item is in index 1 on the global list)
  3. Call RemoveItem(globalIndexOfFilteredItem) on the filtered list
  4. Observe that the filtered list does not update to show that the item has been removed
  5. Reset the filter
  6. Observe that the item which was still shown in the filter is no longer present in the global list

Source Code Please include source code if needed to reproduce the behavior.

Expected behavior When an item is shown as being removed from the filtered list, the same item should be removed from the global list. Likewise, when an item is removed from the global list, that item should be removed from the filtered list if it is present.

Screenshots Add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

g026r commented 3 weeks ago

Workaround I've found for this at the moment is to find the global index of the item being removed, and then call SetItems(slices.Delete(Items(), GlobalIndex, GlobalIndex+1)), which triggers a filter update & results in the same item being removed from both the filtered & unfiltered lists.

It is quite obviously less than ideal. But better than the alternative of "have the correct item removed from the filtered list but have a different item removed from the global list, or have the correct item removed from the global list but different item removed from the filtered list."

prithvijj commented 2 weeks ago

If I understand the issue right: When filtering, if the item is removed from the filtered items, it doesn't remove the same item from the global list,

And ideally it should be the case where: When filtering, if the item is removed from the filtered items, it should also remove the same item from the global list

Is my understanding for it correct? If so feel free to checkout this PR https://github.com/charmbracelet/bubbles/pull/638 if this is what you had in mind related to the issue, and the fix for it

g026r commented 1 week ago

@prithvijj Something like that, yeah. Probably the simplest way to describe it is that it removes the item at that index in both the filtered & unfiltered items. But since they're filtered, the item at that index isn't the same between the two.

I'll take a look at your PR.

tbh, I found it somewhat odd that RemoveItem operates on the filtered list to begin with. SetItem and InsertItem, the other two methods that operate on the a single element in the list, don't modify the filtered list; instead they only modify the full list & then return a tea.Cmd indicating that the filter needs to be updated.