Inline Search: add Jetpack branding + Class Refactor #43330
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add two new methods, init_corrected_query_hooks() and update_search_options_with_correction() - These are likely to be changed - Adjust search-results.jsx to (1) search against the correct query while (2) display informative text - This will be adjusted as the current functionality is to establish a foundation while ensuring things work - Adds new styles for the "Did you mean?" line
- Add proper "Did you mean?" text - Remove code from last week's testing - Prep for turning the search suggestion into a link
- Update styling - Rather than offering a "Did you mean?" choice, automatically try search suggestion
Restoring Instance Search changes back to trunk as our changes are going to apply to Inline Search only.
- Hook into pre_get_posts() for later displaying the corrected_query text
- Add a new setup_corrected_query_hooks() method which is to be modified in a future commit.
- Add a new maybe_use_corrected_query() method
- Add a new prepend_corrected_query_to_first_result() method to ensure that the corrected_query, if it's used, displays in the "Search results for:..." heading
- Add get_corrected_query_html() for outputting markup
- Add JS for injecting our new markup. This will be moved to its own file in a future commit.
Functionally:
- If a spelling error is corrected, and there is a result for the corrected spelling, the search form will return with two headings: (H1) "Search results for... { corrected_query }" followed by (H2) "No results found for... { original_query }"
- If a spelling error is discovered but there is no result for the corrected spelling, the search form will return with one headaing: (H1) "No results found for... { original_query }"
- Refactor output_corrected_query_script() method - New get_title_selectors() method - New enqueue_corrected_query_script() method - Refactor output_corrected_query_html() method - Moved JS to new ../inline-search/js file - Add a new filter to allow users to specify their own search title selectors if their theme does not use one of the three selectors specified. These were checked across themes Twenty Twenty Five going back to Twenty Ten.
- Check $_GET['s'] with null coalescing instead of isset - Favour an early return instead of the ! checks - Change message language to 'No results for %s' - Remove <strong> tags to avoid forcing bold I also looked into an option like inserting <span> tags in case a user wanted to style the $original_query word themselves, but I wasn't happy with the implications of this for translators, and the word can always be styled with a bit of fairly straightforward JS.
- Adds search result highlighting using the same <mark> tag approach as Instant Search. - Splits up filter__pre_get_posts() with more discrete methods to avoid overloading filter__pre_get_posts() by giving it too much to do. - Adds new filters and new methods as appropriate.
Split out search correction methods into their own class
- Leverage a similar approach to Search Correction to inject Search Branding - Test against the 34 most popular Atomic themes - Refactor existing classes to make use of abstraction due to similarity of methods between the colophone and search-correction classes - Update webpack config
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
bot
added
the
[Package] Search
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Search plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCannot generate coverage summary while tests are failing. 🤐 Please fix the tests, or re-run the Code coverage job if it was something being flaky. Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
When the API sends highlighted data back, it's returned without the expected <p> tags that would wrap content. Let's add those back.
github-actions
bot
added
[Plugin] Jetpack
…ing' into add/inline-search-branding
anaemnesis
changed the title
anaemnesis
added
the
Coverage tests to be added later
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
This PR is built on top of #43110, and will be merged after that is merged. That PR should be reviewed prior to this one as this PR includes both the present Jetpack Branding implementation, as well as highlighting. With that in mind, I'm opening this to review as well as eyes would still be helpful with respect to approach.
Proposed changes:
This PR aims to increase parity between Inline Search and Instant Search by adding Jetpack Branding to search results when Inline Search is active.
This PR takes the same JS injection approach as #42473. This will serve as the go-live basis and to demonstrate the implementation for any future iterations, e.g., moving from JS injection to a block-based approach.
Because this class, the search correction class, and the highlighter class use similar methods, I've opted to abstract shared methods to
class-search-component.php. This then avoids quite a bit of duplicated code, though it may require some careful merge conflict resolution.Per TwentyTwentyFive (TT5), the proposed implementation is shown below. I have some reservations about the amount of space between the final result and the branding, but this is a result of TT5 template markup and would require a change to the theme, so, this may be a limitation of the current approach.
As with #42473, this change was tested with the top ~36 themes from Atomic. Please DM me on Slack for access to any of the non
pub/themes if you don't already have access to them for testing purposes.Other information:
Jetpack product discussion
N/A
Does this pull request change what data or activity we track or use?
N/A
Testing instructions:
add_filter( 'jetpack_search_replace_classic', '__return_true', 20 );somewhere (e.g. usingCode Snippetsplugin) or add an earlyreturn truetoshould_replace_classic_search().Jetpack --> Searchin WP Admin.jp watch packages/searchor build after checking out this branch.jetpack_search_colophon_selectorsfilter: