Set cart page is loading while fetching data from network #2454
Conversation
|
|
Performance Test Results The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate. https://pr-2454.pwa-venia.com : LH Performance Expected 0.85 Actual 0.57, LH Best Practices Expected 1 Actual 0.92 |
supernova-at
added
the
version: Patch
There was a problem hiding this comment.
Conceptually I agree that we should always render data from the cache, but I think we should solve for this case. We do have the ability to derive that our cart is empty, and it seems preferable to render the loading indicator for that case. In the instance you have cart items, and a new update from the network, the current behavior is fine. Comes down to what should we prefer:
- Cart not empty - Flash of empty cart before seeing real cart vs loading indicator then cart
- Cart empty - see empty cart immediately vs waiting a little longer before displaying empty cart
I'm also not sure if this needs some new treatment after our meeting on Thursday, so I'm going to move back to UX Review so you and @schensley can review.
There was a problem hiding this comment.
Thank you updating the tests @supernova-at. Other than a minor comment about tests, it looks fine for me. I'll leave the test addition call to you.
| @@ -4,15 +4,14 @@ import { createTestInstance } from '@magento/peregrine'; | |||
| import { useCartPage } from '../useCartPage'; | |||
There was a problem hiding this comment.
Not sure how granular can we get with testing but I feel we have to add tests around conditionals and effects. For instance, check if isCartUpdating is being updated on loading change.
m2-community-project
bot
added
Progress: approved
and removed
Progress: changes requested
labels
|
@supernova-at Looks good other than being on |
Description
This PR sets the cart page's
isCartUpdatingflag while we are fetching data from the network.This will not allow users to "Proceed to Checkout" until we know we have the latest data from the network and the user is seeing the most up-to-date information.
Notably, this PR does not solve the cart page flash problem - the cart page purposefully shows data returned from its cache first, and updates it with data fetched from the network. It purposefully does not show a loading spinner while we wait for network data (that would defeat the purpose of having cached data).
The "flash" is the page showing the cached data first and then switching to the network data.
PWA-321 will properly add items to the cache and actually solve the flash problem, but even then there is a chance the cache and the network are out of sync, and the changes in this PR are useful to have in place regardless.
Related Issue
Closes PWA-416.
Acceptance
Verification Stakeholders
Specification
Verification Steps
/cartpageScreenshots / Screen Captures (if appropriate)
Checklist