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

Move default autosuggest selectors definition so they can be filtered. #2181

Merged
merged 4 commits into from
May 6, 2021

Conversation

JakePT
Copy link
Contributor

@JakePT JakePT commented Apr 21, 2021

Description of the Change

The autosuggest feature is automatically added to these selectors .ep-autosuggest, input[type="search"], .search-field. These are defined in the JavaScript file, and cannot be changed by the user. This PR moves the definition of the default selectors into the script localization so that they can be changed by filtering the defaultSelectors option with the existing ep_autosuggest_options filter.

It also adds a ep_autosuggest_default_selectors filter just for the default selectors.

Alternate Designs

In #2168 @johnbillion suggested a selectors option, which could take precedence over the selector option if it was present. I opted to just put the default selectors into a defaultSelectors option to avoid potential ambiguity between selector and selectors, and possible confusion due to what would be the absence of an existing selectors option in the values that are being filtered.

Another approach could have been to implement the filter in JavaScript, since that's where the original value was, but I opted for putting them inside the PHP filter since that PHP filter was already established as being available to change some selector behaviour.

Benefits

The most obvious use case would be to stop the plugin adding Autosuggest to all search inputs, by removing the input[type="search"] selector using:

add_filter(
	'ep_autosuggest_options',
	function($options) {
		$options['defaultSelectors'] = '.ep-autosuggest, .search-field';

		return $options;
	}
);

or

add_filter(
	'ep_autosuggest_default_selectors',
	function($selectors) {
		$selectors = '.ep-autosuggest, .search-field';

		return $selectors;
	}
);

The existence of an additional filter specifically for selectors enables removing default selectors with just:

add_filter( 'ep_autosuggest_default_selectors', '__return_empty_string' );

Possible Drawbacks

In the event that a user has used the ep_autosuggest_options filter in such a way that does not pass through the defaultSelectors option, autosuggest would no longer be added to the default selectors.

Verification Process

  1. Enable Autosuggest and add a search widget. Do not enter anything into the selector setting.
  2. Confirm that autosuggest is working.
  3. Add code like this to remove the default selectors:
add_filter( 'ep_autosuggest_default_selectors', '__return_empty_string' );
  1. As long as the theme does not add an ep-autosuggest class itself, no autosuggest functionality should be present in the search widget.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Addresses #2168.

Changelog Entry

Allows changing or removing the default selectors used for autosuggest by filtering the defaultSelectors option in the ep_autosuggest_options filter.

@JakePT JakePT linked an issue Apr 21, 2021 that may be closed by this pull request
@JakePT JakePT changed the title Move default autosuggest selector definition so they can be filtered. Move default autosuggest selectors definition so they can be filtered. Apr 21, 2021
@johnbillion
Copy link

I think this makes more sense than my original suggestion of adding a new option 👍

includes/classes/Feature/Autosuggest/Autosuggest.php Outdated Show resolved Hide resolved
assets/js/autosuggest.js Outdated Show resolved Hide resolved
@JakePT JakePT requested a review from Rahmon May 5, 2021 05:20
@Rahmon Rahmon self-assigned this May 5, 2021
Copy link
Contributor

@Rahmon Rahmon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Rahmon Rahmon removed their assignment May 6, 2021
@brandwaffle brandwaffle merged commit 211688c into develop May 6, 2021
@brandwaffle brandwaffle deleted the feature/2168 branch May 6, 2021 16:12
@felipeelia felipeelia added this to the 3.6.0 milestone Jul 5, 2021
@JakePT JakePT mentioned this pull request Jul 14, 2021
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to override the autosuggest selector, not just append to it
5 participants