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

Instant Search: Add infinite scrolling #13684

Merged
merged 13 commits into from
Oct 10, 2019

Conversation

jsnmoon
Copy link
Contributor

@jsnmoon jsnmoon commented Oct 7, 2019

Changes proposed in this Pull Request:

This enables paging via a "Load More" button with infinite scrolling support.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Yes, this adds paging to Jetpack Instant Search results.

Testing instructions:

  • Add define( "JETPACK_SEARCH_PROTOTYPE", true ); to your wp-config.php.
  • Ensure that your site has the Jetpack Pro plan and Jetpack Search enabled.
  • Add a Jetpack Search widget to the Search page sidebar.
  • Enter a query into the search widget.
  • Ensure that the "Load more results" button renders like so:
Screen Shot 2019-10-07 at 1 50 59 PM
  • Scroll to the bottom of the page. Ensure that more results are fetched and appended to the list of search results.
  • Without scrolling to the bottom, click on the "Load more results" button. Ensure that more results are fetched and appended to the page.

Proposed changelog entry for your changes:

None.

@jsnmoon jsnmoon added [Status] Needs Review This PR is ready for review. [Type] Feature Request [Feature] Search For all things related to Search Instant Search labels Oct 7, 2019
@jsnmoon jsnmoon requested review from gibrown and a team October 7, 2019 19:56
@jsnmoon jsnmoon self-assigned this Oct 7, 2019
gibrown
gibrown previously requested changes Oct 8, 2019
Copy link
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

I briefly contemplated resolving the conflicts but I though I think I see how the results are getting concatenated between requests, unsure I understand it well enough to do that merge. ;)

The scrolling works pretty well. Also tested when getting to the end of the results and that works well too.

Somehow though the filtering seems to be broken now. The initial search is not showing any filters. Sometimes they appear when scrolling, but then they disappear after applying some filtering. I assume it has to do with how the results are getting combined.

My other comments are pretty minor and not worth holding up this PR for. I like that you are directly relying on the CSS from the theme for the button. Looks good.

@jsnmoon jsnmoon force-pushed the add/instant-search-scroll-2 branch from f1929e3 to c4c6bbc Compare October 8, 2019 20:29
@jsnmoon jsnmoon force-pushed the add/instant-search-scroll-2 branch from c4c6bbc to a0f724c Compare October 8, 2019 20:31
@jsnmoon jsnmoon added [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Oct 8, 2019
@jsnmoon jsnmoon force-pushed the add/instant-search-scroll-2 branch from 540a623 to 3369929 Compare October 8, 2019 23:23
@jsnmoon jsnmoon added the [Status] Needs Review This PR is ready for review. label Oct 9, 2019
@jsnmoon jsnmoon requested a review from gibrown October 9, 2019 18:44
@jsnmoon jsnmoon dismissed gibrown’s stale review October 9, 2019 18:45

This was due to a silly mix up in a ternary statement -- should be fixed!

Copy link
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

Still has a bug somewhere. When I load more results the filters then disappear. I'm guessing that the aggs results are just getting overwritten by the second search query rather than persisted.

@jsnmoon jsnmoon requested a review from gibrown October 10, 2019 19:11
@jsnmoon jsnmoon dismissed gibrown’s stale review October 10, 2019 19:12

Aggregations should now be preserved for paged responses.

Copy link
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

Working for me now. Ship it!

@jsnmoon jsnmoon merged commit 670243f into instant-search-master Oct 10, 2019
@jsnmoon jsnmoon deleted the add/instant-search-scroll-2 branch October 10, 2019 20:57
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review This PR is ready for review. labels Oct 10, 2019
jsnmoon added a commit that referenced this pull request Oct 23, 2019
* Implement minimal search results and spelling correction (#13365)
* Add filtering display (#13371)
* Fix search result display bugs and make improvements (#13393)
* Add rudimentary support for filtering on post types (#13430)
* Add support for filtering on categories and tags (#13505)
* Add instant search sorting based on the URL (#13377)
* Add support for filtering on dates (#13545)
* Add custom taxonomy filtering (#13605)
* add sort widget (#13614)
* fix many theme incompatibilities (#13602)
* Add infinite scrolling (#13684)
* Add caching to the api requests (#13714)
* Clean up some design bugs/issues (#13721)
* Fix labels for post types when we have them. (#13750)
* Add localization and formatting of all dates (#13748)
* search from any page on the site (#13713)
* Hook up default options (inc. sort) (#13742)
* Add TrainTracks analytics (#13730)
* Create PostTypeIcon component (#13790)
* Upgrade to Preact 10 (#13794)
* Add comments component (#13797)
* Address review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Search For all things related to Search [Type] Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants