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

[iOS & tvOS] FilterViewModel - Cleanup #1412

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Jan 28, 2025

Summary

Resolves: #1246

This PR updates the FilterViewModel to be Stateful. Additionally, all of the filter logic is moved from the FilterView to the ViewModel. Finally, while I was working on the FilterView, I found that #1246 was still present. I've updated the SelectorView and tested this from both the FontPickerView and FilterView and this is now working as expected.


I feel very good about the FilterViewModel change to Stateful. As I understand it, the data functionality should be tied to the ViewModel instead of the view. Please let me know if that is not the case or if there is any reason this view was intentional left as non-Stateful.

The SelectorView is functional in both locations it's called from but not pretty. To make this functional for both the String and AnyItemFilter I had to have the selection as both a State and a Binding. I'm open to feedback if there is a cleaner way to do this but I was trying to accomplish this without having to change the callers too dramatically.

If we decide the SelectorView changes are a wash, I'm perfectly fine with this PR focusing on the FilterViewModel changes instead! Let me know!

Final item, the SearchViewModel didn't have a filter for Letters. I've added that as a drop in copy/paste from the LibraryItemViewModel


Before:

The reset appropriately resets the filter but the UI does not update. This is because the selection in a binding box is not updating correctly. My solution was to have a State item for editing that updated the Binding item on change.

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-27.at.18.51.07.mp4

After:

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-27.at.18.44.13.mp4

@JPKribs JPKribs added bug Something isn't working enhancement New feature or request labels Jan 28, 2025
@JPKribs JPKribs changed the title [iOS] FilterViewModel - Cleanup [iOS & tvOS] FilterViewModel - Cleanup Jan 29, 2025
@JPKribs JPKribs removed the enhancement New feature or request label Jan 30, 2025
…odified. Update the init and update. Hold only the modified filters in `modifiedFilters` instead of `(modifiedFilters, bool)` since that's just clunky and unnecessary.
… should now be handled on the FilterViewModel
@JPKribs
Copy link
Member Author

JPKribs commented Jan 31, 2025

This one should be ready to go. The only item that I was considering adding but didn't was filtering the filters. Sometimes, there is a navigationFilterBar when there are no options for that filter.

A good example of this Favorites. You can look at the favorites but there is no option for Genres or Filters in the dropdown. I would argue if the FilterViewModel returns nothing in AllFilters that dropdown should be hidden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant