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

[5.x] Don't enforce a query length on comb searches #10496

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

ryanmitchell
Copy link
Contributor

@ryanmitchell ryanmitchell commented Jul 23, 2024

Sometimes you want to show users a list of results, and then allow a query to be entered that filters that listing, rather than entering the query before showing the listing.

As things stand that means using the collection tag for the non-query state, and then the search:results tag for the queried state. This is because the Comb search driver doesn't allow blank queries. This isn't ideal and means a lot of code repetition for what is the same thing.

This PR updates the logic to allow blank queries when you have 'min_characters' => 0 in your search index config. This allows you to use the search:results tag for both situations mentioned above:

{{ search:results for="{get:q ?? '*'}" as="results" }}

This shouldn't break any existing installs as min_characters: 1 is the default (?).

I left the NoQuery class in place on the off chance someone might be using it.

}

if ($length < $this->min_characters) {
if (strlen($query) < $this->min_characters) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you should maybe leave it as it was, but change the NoQuery condition to if ($length === 0 && $this->min_characters > 0) {

Then things will work exactly the same for everyone, but if you set your min_characters config to 0, you can go ahead and search for nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea

@jasonvarga jasonvarga merged commit 40d535f into statamic:5.x Jul 23, 2024
16 checks passed
@ryanmitchell ryanmitchell deleted the fix/allow-blank-queries-to-comb branch July 23, 2024 17:24
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.

2 participants