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

Fixed Reset button for Selector view #1247

Closed
wants to merge 0 commits into from
Closed

Conversation

FrankreedX
Copy link

@FrankreedX FrankreedX commented Sep 28, 2024

Closes #1246
Fixes the "Reset" button in the filter selector not resetting the selector itself, only the data outside the selector.
Screenshot 2024-09-28 at 12 46 21 PM

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, this isn't the correct solution. That StateObject is there as an intermediate "binding" to update the view if it couldn't be from a parent view. For example with this change, the font selection breaks. See BindingBox for its justification.

The difference in behavior comes in FilterView and how we perform some indirection wackiness to update the correct filters. Filters are a bit confusing.


I would be able to take a look at this a bit more later but I may just revert the BindingBox's usage from #1216. I only implemented it since I thought I could save some code, but will revert if it's causing more issues than solving them.

@FrankreedX
Copy link
Author

If we want to support both use cases, maybe BindingBox can enable or disable its 2 way binding. From what i can see, BindingBox is a binding but only 1 way right? The other way can be a boolean that is passed in with a default of 2 way, and in special cases use 1 way instead.

@LePips
Copy link
Member

LePips commented Oct 6, 2024

Bindings should ideally/really always be both ways. BindingBox essentially making a one way binding is more of an unwanted side effect and I'll have to take a look at it again, but below is quick fix.

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

Successfully merging this pull request may close these issues.

The "Reset" button in Filters in search doesn't work
2 participants