-
Notifications
You must be signed in to change notification settings - Fork 318
Fixed PHP autosuggest placeholder is not being used. #1619
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
base: develop
Are you sure you want to change the base?
Conversation
@mustafauysal can you review this one? |
@tlovett1 looks good to me 👍 |
This PR will need to be refreshed and have tests added. Setting it to EP 5.3.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue with the autosuggest feature where a custom PHP placeholder value was not being applied in the JavaScript query. Key changes include:
- Removing the redundant placeholder parameter from the buildSearchQuery function.
- Updating the query to use the global placeholder from epas.
- Adding a Cypress test to verify that the autosuggest feature works correctly with a custom placeholder.
Reviewed Changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/cypress/integration/features/autosuggest.cy.js | Added a test case to ensure that the autosuggest feature works with the custom placeholder. |
assets/js/autosuggest/index.js | Removed the unused placeholder parameter from buildSearchQuery and updated the replacement logic to use epas.placeholder. |
Files not reviewed (2)
- .wp-env.json: Language not supported
- tests/cypress/wordpress-files/test-plugins/custom-autosuggest-placeholder.php: Language not supported
Description of the Change
Alternate Designs
const placeholder = 'ep_autosuggest_placeholder';
I considered usingconst placeholder = queryJSON.placeholder;
but decided to drop the line completely as the placeholder is used inbuildSearchQuery()
only.Benefits
Possible Drawbacks
Verification Process
Changelog Entry
Credits
Props @fabianmarz @burhandodhy
Checklist:
Applicable Issues
Changelog Entry