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

Make setting FilteredList.edit.setText('') optional in FilteredList:setChoices? #4983

Open
robob27 opened this issue Oct 5, 2024 · 2 comments

Comments

@robob27
Copy link
Contributor

robob27 commented Oct 5, 2024

I ran into a situation where this line of code was causing me some confusion.

I have a script that has a bunch of tabs that determine the current "view" and "subview". For e.g. Citizens -> Adults, Animals -> Wild. There is a single FilteredList that displays the units matching the filters determined by view/subview + a filter that the user provides in the FilteredList.edit.

image

The FilteredList has an edit_on_change callback that stores the value of the filter when it is changed. The purpose of this is to keep track of the current filter as you switch between views and restore them. So in the above screenshot, if I click away to a different tab and then switch back to Citizens -> Adults, I want the filter to be "adil" again.

When I switch views, I call FilteredList:setChoices(newChoices). This causes FilteredList.edit.setText('') to be executed, which causes edit_on_change to trigger which is causing my filter cache to be updated to '' for that view/subview as well.

I think it should maybe be possible to call this method without resetting the edit value.

One way I can think to work around this for now would be to set some global variable before I call FilteredList:setChoices(newChoices), check the value in my edit_on_change callback to determine if it should be executed, then set the value back after the call.

@myk002
Copy link
Member

myk002 commented Oct 5, 2024

The interaction between the filter and setChoices has been a wart for as long as I can remember. If we change the implementation, then we'll have to go through and change all callers to the API, but it might be a good idea to do that work.

The "best" approach that I've been able to come up with is this sequence of calls:

    local saved_filter = list:getFilter()
    list:setFilter('')
    list:setChoices(self:get_choices(), list:getSelected())
    list:setFilter(saved_filter)

I suspect that this is probably what should happen automatically when you call setChoices, but I haven't checked and tested all the callers to make sure that's the case.

@robob27
Copy link
Contributor Author

robob27 commented Oct 5, 2024

Ah yeah, that's a much better workaround. Thanks!

I was trying to do:

  1. Cache the filter on change, per view/subview
  2. On view/subview change, call setChoices with the new unit choices
  3. Restore the filter from the cache in step 1

This made sense to me because I wasn't accounting for setChoices updating the filter (despite docs lol) and triggering the edit_on_change. So I was seeking a solution that prevented that and was totally missed your much cleaner approach.

Now I am doing:

  1. Cache the filter on change, per view/subview
  2. On view/subview change, store the cached filter locally, then call setChoices with the new unit choices, then restore the filter (thus restoring the cache)
function UnitSearchWindow:setChoices(choices)
  local storedFilter = self:getStoredFilter()-- returns the stored filter for the current view/subview, stored in edit_on_change
  self.subviews.unit_list:setChoices(choices)
  self:setFilter(storedFilter)
end

I think I would still benefit slightly from being able to call setChoices without list:setFilter('') happening since I need to restore the value for the view/subview I'm switching to rather than retaining the current filter value, and FilteredList wouldn't know about that value. Though I think I could still work with what you suggested by doing:

  1. Cache the filter on change, per view/subview
  2. On view/subview change, restore the filter first, then call list:setChoices

I still think it might be slightly unintuitive having to ensure that I restore the filter before calling setChoices to prevent my stored filter from getting messed up when I call setChoices, but it's not too bad to deal with if the alternative is a ton of work with probably little benefit for most cases over what you wrote.

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

No branches or pull requests

2 participants