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

Advanced Filters : Method based Search #1047

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ArjanSM
Copy link
Contributor

@ArjanSM ArjanSM commented May 26, 2023

📷 Screenshots

@cortinico Not sure if this is what you had in mind

📄 Context

Refer #286

📝 Changes

I'm not super proud of it but I'm using Regex to identify the custom search query pattern 🤦
I can elaborate it to enable search on the basis of protocol and other factors but thought of getting this reviewed.

📎 Related PR

🚫 Breaking

None

🛠️ How to test

Unit test included

⏱️ Next steps

@ArjanSM ArjanSM requested a review from a team as a code owner May 26, 2023 17:13
@ArjanSM ArjanSM changed the title Advanced Filter - I method based Search May 26, 2023
@ArjanSM ArjanSM changed the title method based Search Advanced Filters : Method based Search May 26, 2023
@vbuberen
Copy link
Collaborator

Would like to share some feedback as well.
It feels a bit weird to type method search. As a user I would prefer to have some filtering view with checkboxes, like the one I described in the issue you referenced. This way users won't be confused by what they can type in search view and have a visible set of filters right away. With current implementation you showed I feel like most users won't even notice that there is such functionality.

@ArjanSM
Copy link
Contributor Author

ArjanSM commented May 30, 2023

@vbuberen
I agree that discovery of the method search in this pr is something that would require developer education.
Maybe, I misunderstood what @cortinico meant in this comment.
I also understand that you'd wanted to expose the filters using a BottomSheet.
This is what I'd worked on before I thought of implementing the changes in this PR.

@ArjanSM
Copy link
Contributor Author

ArjanSM commented Jul 6, 2023

@vbuberen I agree that discovery of the method search in this pr is something that would require developer education. Maybe, I misunderstood what @cortinico meant in this comment. I also understand that you'd wanted to expose the filters using a BottomSheet. This is what I'd worked on before I thought of implementing the changes in this PR.

Still a Work In Progress. But this is where I'm at.

@cortinico cortinico added this to the 4.1.0 milestone Jul 6, 2023
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.

3 participants