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

Support customizing the template for search results in Instant Results #2959

Merged
merged 11 commits into from
Oct 11, 2022

Conversation

JakePT
Copy link
Contributor

@JakePT JakePT commented Aug 23, 2022

Description of the Change

This implements a JavaScript hook using @wordpress/hooks to support developers replacing the component used to render search results in Instant Results.

In #2527 I described an approach where the filter wraps the return value to filter the rendered element upon render, but in this PR I ended up taking a different approach where I am filtering the component itself. I did this for a few reasons:

  1. The filter only runs once, when the component is defined.
  2. Conditionally replacing the component per-result is still possible via a wrapper component approach, which I've documented.
  3. I think it's easier to understand what needs to be returned from any filter callbacks, as a React component is a well known concept, whereas filtering just the rendered result could cause confusion and give the incorrect impression that raw HTML could be returned to the filter or that other rendering methods would be supported.

Something that was also considered was the naming of JavaScript hooks. @felipeelia and I explored a few other plugins and Gutenberg itself and didn't find much consistency. What this PR proposes is plugin.Feature.valueBeingFiltered In this case that's elasticpress.InstantResults.Result.

The Result value is capitalized to reflect the fact that a React component is being filtered and those are conventionally named, and represented in JSX, capitalized. If the value of a regular function such as getPostTypesFromForm() were to be filtered that would use elasticpress.InstantResults.getPostTypesFromForm.

Some other things to think about are whether the namespace should be elasticpress or ep, to be somewhat consistent with the PHP hooks, and how to capitalize the feature names since they don't represent an actual object in JavaScript. I went with InstantResults over instantResults to match the PHP class and for aesthetic reasons.

Closes #2527

How to test the Change

The included documentation update includes some examples of how the filter can be used. Implementing these would confirm if things are functioning as expected.

Changelog Entry

Added - elasticpress.InstantResults.Result JavaScript filter for filtering the component used for Instant Results search results.

Credits

Props @JakePT

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@JakePT JakePT requested a review from felipeelia August 23, 2022 16:23
@JakePT JakePT self-assigned this Aug 23, 2022
@JakePT JakePT marked this pull request as ready for review August 24, 2022 11:51
@chaselivingston chaselivingston assigned tott and felipeelia and unassigned JakePT Sep 13, 2022
Copy link
Member

@felipeelia felipeelia left a comment

Choose a reason for hiding this comment

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

@JakePT in addition to my comments, we also need to fix a conflict in theme-integration.md. Do you mind handling that? Thanks.

ps.: It would be great if we could have e2e tests checking the new filter as well. Would you be able to add it?

assets/js/instant-results/components/common/result.js Outdated Show resolved Hide resolved
docs/theme-integration.md Outdated Show resolved Hide resolved
@felipeelia felipeelia assigned JakePT and unassigned tott and felipeelia Sep 15, 2022
@felipeelia felipeelia merged commit a10c114 into develop Oct 11, 2022
@felipeelia felipeelia deleted the feature/2527 branch October 11, 2022 11:48
@felipeelia felipeelia added this to the 4.4.0 milestone Oct 24, 2022
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.

Support customizing the template for search results in Instant Results
3 participants