Selecting a Product Variant does not replace the route anymore. #548
Conversation
Pull Request Test Coverage Report for Build 2075
💛 - Coveralls |
| export const productWillEnter$ = routeWillEnter$ | ||
| .filter(({ action }) => ( | ||
| action.route.pattern === ITEM_PATTERN && | ||
| action.historyAction !== HISTORY_REPLACE_ACTION |
There was a problem hiding this comment.
Now that the route isn't replaced anymore, this part of the condition can be removed. The rest needs to stay as it is and can't be substituted by productWillEnter$ from common-commerce, since this one also emits when the route is updated.
| @@ -15,7 +15,7 @@ import { | |||
| RECEIVE_PRODUCT_RELATIONS, | |||
| } from '../constants'; | |||
|
|
|||
| export const productWillEnter$ = routeWillEnter$ | |||
| export const productWillEnter$ = routeWillEnter$.merge(routeDidUpdate$) | |||
| .filter(({ action }) => action.route.pattern === ITEM_PATTERN); | |||
|
|
|||
| export const variantWillUpdate$ = routeWillEnter$.filter(({ action }) => | |||
There was a problem hiding this comment.
The variantWillUpdate$ stream also needs to be adjusted, since the HISTORY_REPLACE_ACTION action isn't dispatched anymore on variant changes. At the moment it doesn't emit anymore, but it's relevant for the variantDidChange$ stream which is used to reset the AddToCartBar and to track variant changes (pwa-tracking/subscriptions/product).
I've quickly tried to update the stream by branching it from routeDidUpdate$, but then variantDidChange$ emitted with the data of the parent product, instead of the variant data.
There was a problem hiding this comment.
This gets triggered now with the correct data.
There was a problem hiding this comment.
The basic theme logic seems to work now, but the variantSelected tracking event doesn't contain the correct data yet. Both properties within the event payload (baseProduct and variant) contain the parent product.
The reason for that is located within variantDidChange$ of pwa-tracking/subscriptions/product. There we grab the productId from the params of the Route state, and use it to select the tracking data.
I currently see two possible ways to fix this. You could either extend the logic to also consider the productId from the state, just take it from action.productData.id. action.productId is also possible when the receivedVisibleProduct$ stream emits for RECEIVE_PRODUCT, but RECEIVE_PRODUCT_CACHED doesn't deliver this property at the moment.
Additionally the tests seem to fail at the moment.
fkloes
deleted the
PWA-1583-Variant-Selection-should-not-unmount-page
branch
Description
Selecting a Variant on the Product Detail Page used to replace the route to trigger loading of the product data. This caused issues with the scrolling of the page. Now the page is not replaced but the current route is updated and the fetching of new data is based in the route's custom state, which holds the selected variant's ID.
Type of change
Please add an "x" into the option that is relevant: