Add Skeleton loading component to the PMME element on the PDP #9166
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +109 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
|
This implementation looks similar to the one in this comment, but it does not flicker when we replace the loader with the PMME. I see we use the same logic of removing the loader and setting the display of the PMME to block. Have you had a similar issue while working on this? |
@gpressutto5, I ended up smoothing this out by giving the PMME element and its contents defined dimensions and adding the loader as a fixed element inside it, so I didn't have to really remove and replace anything. That solved a lot of issues with bits of the DOM jumping around awkwardly, so perhaps that approach could be helpful on the cart as well. That being said, the cart page reloads more readily than the PDP page, so I can't say whether this adds more of the flickering that you're encountering. If that is the case, I can't think of a very good solution from our perspective unfortunately. |
FangedParakeet
requested review from
a team and
brettshumaker
and removed request for
a team
There was a problem hiding this comment.
While working on the same approach for the blocks cart (#9107) I found a few issues:
1. 4rem is not always enough
This example uses:
font-family: 'Verdana';
font-size: 1.25rem;2. empty space when element is not loaded
When the element is not properly loaded (because the price/product is invalid or because there was a server error) we will have an empty blank space. I simulated a server error by adding 127.0.0.1 m.stripe.network to my hosts file.
After refactoring the styles, I was able to remove the hidden overflow and used
I raised that here -> #9126 (comment) ...and I think we're fine with this. Comparing the approach of other competitors, it seems somewhat standard to just leave this space empty if we are unable to display a PMME (see here -> #9126 (comment) )
@gpressutto5, made some fixes in 03caf47. Let me know if these compromises sound reasonable to you and if this does manage to solve a few problems for you on the cart-side implementation. 🙏 |
|
@pierorocca, following your feedback in #9107 (comment) ...
...should we make the width of the skeleton on the PDP fill the full width of its container or keep it closer to the width of its contents? Examples below. Screen.Recording.2024-07-31.at.2.59.23.PM.movFull width skeleton above. Screen.Recording.2024-07-31.at.2.59.44.PM.movAnd the shorter, PMME-sized skeleton above here. Thoughts? |
|
@FangedParakeet the latter of the two options. Nice work all collabing on optimizing this experience. |
Awesome! That's what is implemented here. Onwards and upwards we go... |
Co-authored-by: Brett Shumaker <brettshumaker@gmail.com>
Fixes #9126
Changes proposed in this Pull Request
Skeleton loader for BNPL PMME on PDP
To prevent the page from jumping when the BNPL PMME is loading on the product details page, this PR adds fixed dimensions to the PMME container and places a skeleton loading animation in place of the PMME, while it is still loading.
Testing instructions
Screen.Recording.2024-07-26.at.18.56.17.mov
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge