Disentangle filter types + library counts + selections

[josiah-roberts]

Based on #780

Why

Currently, the active filter state intermingles:

• Filter type implementation: mapFn, name, etc.
• Library data: count of items with various filter values
• UI data: selected filter types and options

Because of this, even starting on #776 isn't really feasible.

What

This PR separates out:

• Filter type implementation: FilterService.filters + FilterService.filtersMap
• Library data: FilterService.filterState.filterValueCounts
• UI data: FilterServie.filterState.selectedFilters

What else

This also fixes an odd issue where the leftward filters impact the counts on filters to their right, but not vice-verse. Now filters all report all values in the date-filtered photo set. If this is intended, I can update it to be consistent instead of left-to-right. This was intended - I just misunderstood the desired behavior.

• Revert change to intended left-to-right filter behavior

Additionally, I've addressed a number of inconsistencies and type issues - removing casts and making mapFn always return string[] | undefined.

[josiah-roberts]

I've pulled this out of the service class itself so that I can derive type declarations from it.

[josiah-roberts]

This filters in a single pass over c.media. I don't love encoding some of the filter logic here though - e.g. "if there are no keywords, leave it in." I'd love to factor this out into the individual filters themselves.

[bpatrik]

This also fixes an odd issue where the leftward filters impact the counts on filters to their right, but not vice-verse.

This is work as intended. The filters bar is actually a copy from lightroom and tries to mimic the same behavior.
You can think of the filters as pipes, going from left to right.

[bpatrik]

I find it hard to review this PR.

Can you let me know if this change does any:

1. Behavioral change
2. Any UI change?

Do I understand correctly this is only a refactor and should not have any changes?

Can you help the review with the following:

1. Write a test where its clear that this change only does a refactor? (maybe send in a separate PR so that we can see it passes with the current code?)
2. Or at least send this PR in smaller chunks so that is easier to parse

If there is a behavioral and or UI change. Please elaborate on them also include a screenshot that clearly shows the difference.

Sorry but after work I find it hard to gather the brain capacity to review complex PRs.

[josiah-roberts]

Do I understand correctly this is only a refactor and should not have any changes?

Yes, that's correct with the exception of the left-to-right filter behavior, which I didn't understand was intended. I'll revert that change, and there shouldn't be any other behavior changes - it's purely intended to split apart filter behavior, selection data, and library state data.

Can you help the review with the following:

1. Write a test where its clear that this change only does a refactor? (maybe send in a separate PR so that we can see it passes with the current code?)
2. Or at least send this PR in smaller chunks so that is easier to parse

Happy to put up a test PR - do you have a preference between an E2E test vs. a unit test around the filter service?

Sorry but after work I find it hard to gather the brain capacity to review complex PRs.

No worries - thanks for maintaining the project!

[bpatrik]

Happy to put up a test PR - do you have a preference between an E2E test vs. a unit test around the filter service?

Thank you. I fine with any of them.

Edit: unit tests are usually more stable. E2E depends on a lot of external pieces (like the sample gallery).

[bpatrik]

I also made some change in this space. See latest commit. I expect it to be an easy merge

[bpatrik]

I will reject this PR in the following days due to lack of activity.