Skip to content

Conversation

@grimasod
Copy link
Contributor

@grimasod grimasod commented Dec 9, 2019

Related issues

closes #

Short description and why it's useful

Fixes multiple Breadcrumb issues that are new in v1.11

  1. Issues with the Breadcrumb Category filters: there are cases when categories have already been loaded and saved to the category map, but do not match with the breadcrumb filters. In these cases we need to run the category search query without using the category map.

A specific example is Magento's "Default Category". Often every product belongs to this category, but it may not be wanted in the breadcrumbs. It can be excluded by turning off "Include in menu" and adding "breadcrumbFilterFields": { "include_in_menu": true } to the config. This fix is required for this behavior to work correctly.

  1. Breadcrumbs on non-catalog pages: The "routes" prop on the Breadcrumbs component had been removed from pages like Compare and My Account, as they only need the new "with-homepage" prop and the existing "active-route" prop. But without the "routes" prop, the catalog breadcrumbs can appear where they shouldn't, so the "routes" prop needs to be restored.

Screenshots of visual changes before/after (if there are any)

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

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

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

@andrzejewsky andrzejewsky self-requested a review December 9, 2019 21:41
@grimasod
Copy link
Contributor Author

Description updated. This PR now also fixes an issue with the Breadcrumbs component.

@grimasod grimasod changed the title Breadcrumbs Filter fix Breadcrumbs Filter and Component fixes Dec 11, 2019
@andrzejewsky
Copy link
Contributor

andrzejewsky commented Dec 12, 2019

@grimasod I don't fully understand the second issue. When you pass an empty array to the routes you will get eg. Homepage / Compare instead of Homepage / All / Women / Tops / Compare always, no matter if you are on category page. So I don't think so it was an issue...

@grimasod
Copy link
Contributor Author

@andrzejewsky Until now, it has always been Homepage / Compare, so I was just returning it to that.

In the example you give, it would make sense if we had just browsed the Tops category, but not in all cases. For example, if we added some bags to the Compare list and then looked at women's tops without adding any. On the Compare page, we would get Homepage / All / Women / Tops / Compare, even though only bags are displayed.

The other cases too:

  • Homepage / All / Women / Tops / Order confirmation
  • Homepage / All / Women / Tops / My Account
  • Homepage / All / Women / Tops / About Us

Should be

  • Homepage / Order confirmation
  • Homepage / My Account
  • Homepage / About Us

@pkarw pkarw requested a review from patzick December 13, 2019 04:59
@pkarw
Copy link
Collaborator

pkarw commented Dec 13, 2019

I guess we need @patzick comment on this one

@andrzejewsky
Copy link
Contributor

@grimasod ok I see, I could even reproduce it

@andrzejewsky andrzejewsky merged commit a70941b into vuestorefront:develop Dec 17, 2019
@grimasod grimasod deleted the bugfix/fix-breadcrumbs-filter branch January 6, 2020 02:40
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.

3 participants