-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
Supporting multiple filter conditions per field #155
Comments
Hi @masiamj, thanks for the issue. I think you're right, and you already found the place where it goes wrong. I'll try to look into this on the weekend, unless you're up for opening a PR. |
Hey @woylie! I'm a little swamped this week with some personal things so I doubt I'll have the time to make the update before the weekend, but will link a PR here if I do. Otherwise, I'm happy to discuss any ideas if you start looking into it. Thanks a ton for your efforts maintaining this – great work! |
I was able to look a bit further into this, and it appears that the problem actually lies in the flop_phoenix/lib/flop_phoenix/form_data.ex Line 145 in 123e1fe
I didn't find the time to fix this yet, but I should be able to get back to it in the next couple of days. |
Scratch that. I updated the tests with assertions on the value inputs, and they pass. The function I linked would only be problematic if two filters for the same field with the same operator would be used. I don't think that's a use case (if it is, let me know). The zip you linked should be fine for static forms, since the number of filters returned in the meta struct should always match the number of fields in the options passed to the I'll need to test this in an actual application. |
Hey @woylie thanks for looking into this! First off, in my specific case, you're correct to say the same field with the same operator is not a desired use case. Taking a deeper look at the code, I also think you're right that the My new hypothesis is that the issue actually lies in the If you think this is a valid idea I'm happy to help explore solutions. Any thoughts here? Thanks! |
I just tested it in a real-world application, and it works fine. Are you actually having min/max age filters in your form? Or do you by any chance have something like a min/max date or date time? One thing to look out for is that the Flop filter value will be the string value as produced by the HTML input, and it will be passed back as it is to your input component. I had a case with a start date and an inclusive end date, both represented by date inputs. The DB column was a date time, not a date, and before passing the end date to the query function, I added |
Interesting, thanks for checking this out. Maybe this is just an issue with my implementation. I use string, select, and date filters, maybe my form fields below will provide better context:
I don't believe I'm encountering the issue you mentioned as what I'm seeing on submit is a field mismatch in that the |
Are these two different columns? Or should this both be |
^Sorry yes edited to both be price, that was a bad config. My mistake. |
Does it work after changing it? |
Unfortunately not (sorry I copied that from an experiment I was trying, the initial implementation is still having an issue). In an effort to provide as much helpful debugging information as possible, I'm copying the LiveView implementation and resulting filters, hopefully this will help. LiveView:
Params on
This is the output with a single filter on the |
In the code you posted, it still says |
Flop will not render inputs for fields that are not filterable. |
You are completely correct, I don't know how I missed it (think this was a bad merge), but with the change to I'm so sorry for any stress/confusion I caused here, thank you so much for your amazing library and help. |
No worries! |
Is your feature request related to a problem? Please describe.
Hi @woylie, first off, thanks so much for maintaining this project. It's fantastic and I've really enjoyed the API you expose.
The problem: I'm building a relatively static form where I'd like to allow users to enter a min and max range for a specific value. The interface for this would provide 2 separate inputs per field.
EDIT: I believe this problem is well-described here: #88. There's a mention to PR #89, but I'm not sure if this issue ever got resolved?
As an example, let's say we'd like to enter a minimum and maximum age, I believe the field definitions would look like:
From my implementation efforts and research, it appears this structure with the same field and different operators isn't yet supported by the library.
In researching this issue, I found the work you did to use separate labels for multiple fields, and stumbled across this conversation as well which I believe is pertinent.
Right now, I believe the
age
fields in the example would appear separately in the UI; however, upon collating the filter values, they would be merged, taking the value of the firstage
filter, the second one being discarded, and values from the UI being assigned to different inputs via the zip-based implementation here.I could be totally wrong here, if you have any advice to support the use case above, it would be extremely helpful!
Describe the solution you'd like
My ideal solution would be that this library supports an arbitrary number of independently configured inputs per field just based on the
fields
configuration described above.Describe alternatives you've considered
As an alternative, I tried creating a more manual implementation of the
filter_fields
component as described here, but was unsuccessful.Additional context
As I mentioned, I could be totally off base here, but I've been struggling with this for a few hours and would love any advice.
Thanks so much!
The text was updated successfully, but these errors were encountered: