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

fix(storefront): STRF-4875 Fix for sort query parameter with faceted search. #1232

Merged
merged 1 commit into from
May 17, 2018
Merged

Conversation

Ubersmake
Copy link
Contributor

@Ubersmake Ubersmake commented May 10, 2018

What?

The sort search parameter was being cleared in faceted search (product filtering) after a range facet was set or updated.

This issue appears if a sort is done before applying a range. It does not appear if a sort is applied after applying a range, although updating a range will cause the issue to appear.

The root cause is a collision in the template on "sort", which might be looking at a function with the same name rather than the url/search parameter.

Tickets / Documentation

Screenshots

Before:

before

After:

after

@bigbot
Copy link

bigbot commented May 10, 2018

Autotagging @bigcommerce/storefront-team @davidchin

@@ -23,11 +23,11 @@ <h5 class="accordion-title">
<div id="facetedSearch-content--{{hyphenate facet }}" class="accordion-content {{#unless ../start_collapsed }} is-open {{/unless}}">
<form id="facet-range-form" class="form" method="get" data-faceted-search-range novalidate>
{{#each current_selected_items}}
<input type="hidden" name="{{ param_name }}[]" value="{{ param_value }}"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

This could actually be problematic. What param_name values actually come out here? I can verify that none are treated as an array, but if any are actually supposed to be arrays, this change will break that.

Copy link
Contributor Author

@Ubersmake Ubersmake May 10, 2018

Choose a reason for hiding this comment

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

I'll undo that change because other things are using []. But for the record,

Without touching range I can get a url parameter string like:
?_bc_fsnf=1&brand[]=36&brand[]=35&Color[]=Black&Color[]=Blue

Which has identical results to:
?_bc_fsnf=1&brand=36&brand=35&Color=Black&Color=Blue

Values aren't being added to the parameters in any sort of array-like structure. Their keys are simply repeated.

Specific to changes in range.html, without the brackets:
?brand=37&brand=36&search_query=&min_price=1&max_price=99

returns the same results as pre-change:
?brand[]=37&brand[]=36&search_query=&min_price=1&max_price=99

@bc-vandana
Copy link
Contributor

Cool. Thank you for fixing this @Ubersmake

@mattolson mattolson merged commit 4bbd46c into bigcommerce:master May 17, 2018
@Ubersmake Ubersmake deleted the STRF-4875 branch May 17, 2018 16:56
@anilgautamm
Copy link

anilgautamm commented Oct 2, 2018

@mattolson @Ubersmake @bigcommerce/storefront-team @davidchin There is another problem with product filtering when you keep the price filter in the top, and color or other filters in the bottom, when we select a color filter and then select the price range filter, it automatically clears the other filter other than price filter.
https://www.useloom.com/share/f3bcb360539243beb01b6488d21f9e08

@junedkazi
Copy link
Contributor

@anilgautamm can you please create an issue in the issue queue so it is looked into.

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.

7 participants