z-edit / zedit

An integrated development environment for Bethesda Plugin Files.
https://z-edit.github.io
MIT License
304 stars 55 forks source link

listView Filter Improvements #254

Open MarkKoz opened 2 years ago

MarkKoz commented 2 years ago

Summary

Two major enhancements have been made to the filtering capabilities of listView:

  1. Pressing Enter cycles through selection of all matches. If it reaches the last match, then it cycles back to the first match.
  2. A box can be checked to only show items that match the filter. This can be freely toggled on and off.

Implementation

The first feature is fairly straight forward to implement. However, the second added significant complexity. Some things I had to do:

Known Issues

Angular Filter Triggers Unnecessarily

Something I cannot figure out is why the Angular filter is triggered (sometimes twice in a row) for seemingly unnecessary actions e.g. just clicking on an empty space in the list or selecting an item. It happens even when items is the only parameter to the filter function. This is only evident if a breakpoint is set in that function.

This Angular filter may be redundant anyway. Perhaps it's feasible to manually update an array of filtered items. After all, it needs to update when either the items change, the filters change, or the toggle changes, and there are event handlers/watchers for each of those already.

Possibly Redundant Call When Array Updates

The watchCollection calls $scope.filterChanged(), but in retrospect I am not sure if that's necessary. The original line of thinking was that the array may be updated by inserting or deleting an item somehow, in which case the filters would have to be re-applied and a new first index computed.

Selected Item May Be Out of View

When toggling only showing matches, it does not scroll to the selected item again, meaning it may go out of view. Since the currently selected index is not tracked, there's no way to know where to scroll to. It could do a linear search for the first selected index, but not sure if that's worth it. Always tracking the index as a variable seems annoying since it has to get converted sometimes like the previous index is currently converted.

alandtse commented 2 years ago

Thanks for doing this. It's a feature zedit has needed for a while. I hope it gets merged.

For example (to help encourage a close on them if it does get merged):

222

213