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

Hotfix: fixed category filters reset functionality #2264

Closed
wants to merge 1 commit into from
Closed

Hotfix: fixed category filters reset functionality #2264

wants to merge 1 commit into from

Conversation

vue-kacper
Copy link
Contributor

Related issues

#2262

Short description and why it's useful

Fixed category filters reset functionality (on mobile)

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

@pkarw
Copy link
Collaborator

pkarw commented Jan 19, 2019

@vue-kacper look at this one
https://github.com/DivanteLtd/vue-storefront/blob/6c100f978aa79975e4db22be3cefa7f8d38b4c97/core/pages/Category.js#L205

Works with the same usage of the same getter. Seemingly the problem is because in this case getCurrentCategoryQuery.searchQuery is an empty object.

I mean - if this is true this fix can be misleading and it would be better to have explicitly:

searchProductQuery: {}

Do you know what I mean? I mean that searxhLroductQuery is probably a property of the getter value - so by solving this issue this way you’re doing potentially a circular refeeence which could be misleading for other deva reading the code @patzick was refactoring the getters in here so may have something to add - as the problem was introduced with this refactor (I mean it works in 1.6)

Thanks for such a quick fix!

@vue-kacper
Copy link
Contributor Author

@pkarw Aww.. you are right, it would be much better to fix it at getter level

@patzick I'm leaving it to you then :)

@patzick
Copy link
Collaborator

patzick commented Jan 24, 2019

thanks @vue-kacper , i'm closing this in favor of #2282 and will apply this on develop also

@patzick patzick closed this Jan 24, 2019
@pkarw pkarw added this to the 1.8 milestone Feb 2, 2019
@patzick patzick removed this from the 1.8 milestone Feb 7, 2019
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.

None yet

3 participants