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

Search content when at least 3 letters typed in ContentSearch component #231

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

vishalkakadiya
Copy link

Description of the Change

Adds functionality to search the content when at least three letters are typed in the ContentSearch's component search functionality, so we can save network bandwidth.

How to test the Change

  • Add the "Post Searcher" component to the page.
  • Type just one letter the REST API is firing, so it is good practice to fire a REST request when at least three letters are added in the search.

Changelog Entry

Added - Add functionality to search the content when at least three letters are typed in the ContentSearch's search component.

Credits

Props @vishalkakadiya

@vishalkakadiya vishalkakadiya added the [Type] Enhancement New feature or request label Jun 16, 2023
@Antonio-Laguna
Copy link
Member

Hey @vishalkakadiya thanks for raising this!

While I sort of agree with your point, this would be a breaking change since usage could be relying on this. IMHO in the admin scenario in which lots of things are requesting data without you even knowing it's not the end of the world for this to happen.

I think as it stands we wouldn't go with this change. One way I can see to make this potentially work is by adding a prop. Something like: minQueryChars which by default is 0 so it would match but you'd have control to make that number 3 or whatever the number makes sense for your site.

Thoughts?

@vishalkakadiya
Copy link
Author

@Antonio-Laguna Thank you for quickly checking on this. 🙌

Yes, it totally makes sense to build this as the props, let me know if you want me to update this PR to work with that prop.

Thanks! 🙂

@Antonio-Laguna
Copy link
Member

@vishalkakadiya please go ahead! Thanks for be willing to tackle this!

@fabiankaegy fabiankaegy marked this pull request as draft July 28, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants