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

Search Results: fix navigation issues #1641

Merged
merged 1 commit into from
Feb 18, 2020

Conversation

jbruni
Copy link
Contributor

@jbruni jbruni commented Feb 16, 2020

What?

This PR contains a number of bug fixes related to the Search Results page navigation.

If fixes #1170 and #1312 by separating the current navigation page between "Products" and "News & Information" tabs. It becomes possible, for example, to be at "page 3" in one tab, and at "page 1" in the other tab.

It also improves the fix to #1442, provided by commit 5322976. Now, the back button also works when there is a switch between "Products" and "News & Information" tabs, when navigating backwards.

And it also fixes the dynamic loading of search results for the "News & Information" tab. The AJAX load/update only worked for the "Products" tab. Now, it works also for the "News & Information" tab.

It also fixes the annoying "re-load" of the search results immediately after the first load of the search results page.

While working in this PR, another search results navigation issue was found, but the problem is at stencil-utils. It has been reported at bigcommerce/stencil-utils#110 (comment) and a PR for the fix has been submitted at bigcommerce/stencil-utils#117


Technical Details

In this section, I will explain the code changes in this PR.

1️⃣ Changes to search.html template:

  • We already had {{> components/search/product-listing}} for the products results and {{> components/search/product-count}} for the products count, as partials for the Products tab.

  • Now, we introduce {{> components/search/content-listing}} and {{> components/search/content-count}} partials for the News & Information tab results and count.

  • Note that we always want to render the container elements, in order to support toggling between a "zero results page" and a "page with results". We can't keep rendering the containers conditionally (only if there are results) at this upper level (search.html)... if the containers are not rendered, when receiving non-empty search results from a future AJAX call, we are not be able to update the DOM.

2️⃣ New content-count.html partial and changes to product-count.html partial:

  • As part of the strategy of keeping the page number of each tab separated, we update only the respective section (content or product) when updating the DOM. In other words: when the product section is updated, the content section will NOT be updated; and when the content section is updated, the product section will NOT be updated. Makes sense.

  • To achieve the expected results, we output the search url and count values as data attributes in a dummy wrapper span element, for later retrieval and usage in the JavaScript code.

3️⃣ New content-listing.html partial:

  • This has been extracted from search.html to its own partial, so it is possible to update it dynamically.

4️⃣ Changes to search.js code:

  • Let's see the changes to initFacetedSearch() first:

    1. We add jQuery wrappers to the "News & Information" listing and count elements.
    2. We include the new content-* components in the list of partials to retrieve.
    3. In the callback which processes the AJAX call results, there are three changes:
      • We added updates for the two new content-* partials;
      • We only update the content or product elements, according to the requested section (instead of updating everything);
      • We call the showProducts or showContent method, to switch to the proper tab; this ensures the proper tab is displayed after using the back button to browse backwards.
  • The changes to showProducts and showContent are similar. Note that these functions do two things: update the UI, showing the product or content tab; and they also trigger the dynamic load/update of results.

    1. We introduced a boolean navigate parameter (with default to true, to keep the previous behaviour). This way, it is possible to "separate" the two things that these functions do. If we want to switch the UI tab only, without doing AJAX request to load contents, we can do it by passing false as argument to the navigate parameter.
    2. When navigate is true, instead of using the window.location.href, which can point to a different tab, we are looking into the data-url and data-count attributes introduced to the *-count partials. The query string parameter for section will be always correct. And, if the count is zero, we force the query string parameter page to 1. (The use case here is when the initial load was for a page greater than 1, and the tab didn't contain results for that page.)

These are the minimal changes to make the existing navigation in a working state. There is room for further improvements.

Tickets / Documentation

Screenshots (if appropriate)

N/A

@bigbot
Copy link

bigbot commented Feb 16, 2020

Autotagging @bigcommerce/storefront-team @davidchin

@junedkazi
Copy link
Contributor

@jbruni thank you for fixing this issue.

@junedkazi junedkazi merged commit c5f9d5f into bigcommerce:master Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants