-
-
Notifications
You must be signed in to change notification settings - Fork 307
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
base: main
Are you sure you want to change the base?
Conversation
…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
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 |
While it is nice that things are moved out of I'll have to think about the overall architecture of this. |
Summary
Resolves: #1246
This PR updates the
FilterViewModel
to beStateful
. Additionally, all of the filter logic is moved from theFilterView
to theViewModel
. Finally, while I was working on theFilterView
, I found that #1246 was still present. I've updated theSelectorView
and tested this from both theFontPickerView
andFilterView
and this is now working as expected.I feel very good about the
FilterViewModel
change toStateful
. As I understand it, the data functionality should be tied to theViewModel
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 theString
andAnyItemFilter
I had to have the selection as both aState
and aBinding
. 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 theFilterViewModel
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 theLibraryItemViewModel
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 theBinding
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