Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
173 stars 69 forks source link

Skeleton loader on Cart PMME is absent #9107

Closed pierorocca closed 1 month ago

pierorocca commented 2 months ago

In #8606 a skeleton was applied to the PMME in order to prevent screen shift and to provide a loading state to the BNPL offer which loads after the page in painted. I'm observing that the skeleton is missing from shortcode and Blocks sites.

mdmoore commented 2 months ago

@pierorocca Just tested out the cart on the develop branch and I'm seeing the skeleton still in place on cart update. Let me know the site on which it's not working and I'll take a look.

I'm observing that the skeleton is missing from shortcode and Blocks sites.

The skeleton is only present on the shortcode cart since the PMME isn't removed from the page in the cart block.

pierorocca commented 2 months ago

Thanks @mdmoore.

Shortcode: https://pieroatomic3.wpcomstaging.com Blocks: https://proccaatomic.wpcomstaging.com

pierorocca commented 2 months ago

https://github.com/user-attachments/assets/9ec400ec-0505-4b4f-aa81-416783f7e7b4

https://github.com/user-attachments/assets/b633fb30-15f0-4c91-824b-105dd48086e6

mdmoore commented 2 months ago

Thanks @pierorocca! Was the skeleton loader implemented on PDPs? I think it was only present on shortcode carts. @brettshumaker might know better, but I don't remember ever seeing it on PDPs. I do see the layout shifts on page load though, so maybe this issue can serve as a task to implement skeleton loading on PDPs.

pierorocca commented 2 months ago

Was the skeleton loader implemented on PDPs?

Am I looking for it but can't find it and any PDP related demos are static images. Maybe I missed it but it's a lot more noticeable on the PDP than the Cart no?

pierorocca commented 2 months ago

Or maybe when computed styles got applied, it slowed down the paint enough that it's now a noticeable issue?

pierorocca commented 2 months ago

PMME isn't removed from the page in the cart block

True for updates, but on page load? There's a lot of screen shifting happening.

FangedParakeet commented 1 month ago

Please add your planning poker estimate with Zenhub @gpressutto5

gpressutto5 commented 1 month ago

@pierorocca I'm adding the skeleton loader to the blocks cart when it's updated. Does it resolve this issue?

Jul-25-2024 14-03-09

pierorocca commented 1 month ago

Needs to be there on page load as well to prevent screen shifting. It's a better UX to not have things moving around.

gpressutto5 commented 1 month ago

@pierorocca Like in the shortcode cart implementation in #8606, we can't have the skeleton in the initial load because we don't know how large it will be, so we wait for it to be rendered once and save its size. It might not even be displayed. This is out of our control because it is an iframe rendered by Stripe. We could analyze the available data and settings and try to predict whether it is going to be displayed and try to calculate an approximate height, but inevitably, we will have things moving around when we don't get the estimated height correct. It would look like this (the proceed button is moving, but it doesn't look like that because of the flicker):

Jul-25-2024 15-26-32

pierorocca commented 1 month ago

Good point. Any other option to get the offer before the page is painted or to make the fetch faster?

gpressutto5 commented 1 month ago

Unfortunately, no. The PMME is out of our control and can only be loaded in the front end. Samir added an initial loader in #9166. It is similar to the one above, but it doesn't flicker. He also had the same problem of not knowing the correct height and had to estimate it until the component loaded for the first time. Do you think it is better to have this loader with an approximate height or not have the initial loader?

pierorocca commented 1 month ago

Correct me if I'm wrong, the PMME will either return nothing because the shopper doesn't qualify (country, currency, total amount) or it will return the logos on the first row and copy on the 2nd row. The copy is wrapped if the container is too narrow. There are also differences between mobile and desktop widths. So there's predictability here of what could be provided in the response.

One approach used by American Eagle is to reserve the max space needed, and if there's an offer it populates otherwise it's left blank.

The other approach I've seen is to shift down once the offer is available with the offer populating a bit more quickly than what I'm seeing from the PMME (are we preconnecting?) and there's only a single, acceptable screen shift down. @FangedParakeet indicated he's trying to minimize shift but that all of the elements we're introducing on the screen are added to the DOM independently so each is causing shift independently. Could you take a look at that work and see if it can be further improved upon?

gpressutto5 commented 1 month ago

@pierorocca I updated it to use the same logic as Samir used:

Jul-30-2024 13-54-18

pierorocca commented 1 month ago

That's great! @nikkivias for QA. Nikki we're trying to limit screen shifting and buttons and bnpl messaging from jumping around independently of each other. Does the skeleton help or hinder here? It was originally implemented but suffered regression so we've brought it back with tweaks.

pierorocca commented 1 month ago

@gpressutto5 could you enabled Google and WooPay please?

gpressutto5 commented 1 month ago

@pierorocca Sorry, WooPay is not set up in my environment. I recorded this video with Apple Pay! Let me know if you need me to set up WooPay to record a new video.

Jul-30-2024 18-42-30

pierorocca commented 1 month ago

@FangedParakeet what do you think?

pierorocca commented 1 month ago

Here's the current 8.0 experience for comparison:

https://github.com/user-attachments/assets/163824d9-c9ea-468d-bd0b-50da07108083

pierorocca commented 1 month ago

@gpressutto5 I think your changes looks better in comparison? Is the slow Apple Pay rendering due to your location or local dev?

FangedParakeet commented 1 month ago

@FangedParakeet what do you think?

Here's what @gpressutto5's changes look like on my local with WooPay & GPay enabled as well.

https://github.com/user-attachments/assets/278205e5-168b-4df7-9671-5c6aab309a1c

There's a bit of extra spacing around the PMME, but the loading of that specific element looks smooth enough. The express checkout buttons are still making the page leap around quite erratically, but possibly there's potential to implement a similar preloader in a separate issue to improve that.

pierorocca commented 1 month ago

Thanks for the video! I slowed it down to .25 speed to get a better idea of what's going on. There's still too much happening even with these improvements.

  1. Feedback for the Blocks component/team is the cart component transition is harsh. It doesn't appear to pulse, and then there's too long of a period between when the loader is removed and when the cart is rendered. That makes it feel choppy. The total and proceed to checkout button are also rendered (3rd image) a bit before the cart summary and in the space that will be occupied by the cart summary, adding to the flurry of movement and choppiness. Are we influencing that or is that Blocks behavior?
  2. I like that the PMME loader loads at the same time as the cart. Let's make the skeleton occupy the full width of what it could take up. Looks cutoff and unbalanced.
  3. Agree on adding a separate issue for the buttons.

image

image

image

image

image

image

image

gpressutto5 commented 1 month ago

I updated the width and fixed an issue I found with the padding on small screens:

image

image

pierorocca commented 1 month ago

@nikkivias question to you if we should have a skeleton loader for the PMME and prevent screen shift or ditch the loader and have the PMME smoothly shift the screen down?

nikkivias commented 1 month ago

@pierorocca @gpressutto5 Better to use a skeleton loader, engagement is enhanced when users see that content is about to load. However, the skeleton should closely resemble the layout of the fully loaded element to manage expectations correctly.

So something like this, and make sure the height doesn't shift

pierorocca commented 1 month ago

That looks sweet. Ty!

gpressutto5 commented 1 month ago

@pierorocca @nikkivias We don't know if the body text will have 1 or 2 lines after loading, so I made the bottom loader skeleton slightly bigger. What do you think?

Aug-07-2024 13-43-37

Aug-07-2024 13-44-58

pierorocca commented 1 month ago

True, can you show what it would look like with one line only?

gpressutto5 commented 1 month ago

@pierorocca The second gif has only one line of text in its body. The first gif has two lines.

pierorocca commented 1 month ago

Ah gotcha one line of text plus the line of logos. I believe if there's only 1 logo, both logo and text are on one line?

image

image

gpressutto5 commented 1 month ago

Aug-08-2024 13-30-01

Aug-08-2024 13-32-08

pierorocca commented 1 month ago

So net we could have 1 to 3 lines and we don't know in advance, is that an accurate summary? @nikkivias fyi.

gpressutto5 commented 1 month ago

Yes, that's correct. To be fair, we can potentially have more than 3 lines if the site uses a wide font with a large font size, but that's extremely unlikely as many other components would look broken.

nikkivias commented 1 month ago

@gpressutto5 Can you tell me what is the minimum and maximum height of the widget once its loaded? I want to create a loader design that will account for both scenarios and anything in between.

gpressutto5 commented 1 month ago

@nikkivias

TLDR: most likely 12px - 70px, but it can be anything.

We can't say, because it uses the style from the website. Here are some variations I could reproduce using Storefront just by updating the price or the number of methods available, except for the last one. However, there are infinite possibilities by changing the theme or updating the font style (as in the last example). It can be any size, but most likely >12px as it would be too small to read if it were smaller. As for the maximum height, it might be around 70px, but there's no limit. If the website uses a wide font with a large font size it could be larger than that (as in the last example).

Text height iFrame height (with margin/padding)
image 16px 26px
image 28px 28px
image 36px 44px
image 36px 44px
image 39px 47px
image 60px 68px
image 42px 50px
image 60px 68px
image 68px 76px
nikkivias commented 1 month ago

@gpressutto5 you shared a gif of how it works on Slack:

Aug-14-2024 14-20-44

The shifting doesn't look problematic to me because it is alleviated by the smooth motion. Unlike if it were to suddenly just appear, which would be jarring. Since there will be some cases where nothing will be loaded at all (in cases where the cart totals doesn't qualify) I am inclined to propose that we don't use a loader at all. Just need to make sure the appearance of the widget is smooth.

Thoughts? @pierorocca

pierorocca commented 1 month ago

Just need to make sure the appearance of the widget is smooth.

@nikkivias I agree with this. After seeing the loader and not being able to predict the right sizing, it made me rethink if it's needed at all. The smoothness is key so that it feels intentional.

gpressutto5 commented 1 month ago

@pierorocca @nikkivias

  1. We use this same loader on the shortcode cart and product pages. Should we remove them as well? cc @FangedParakeet @mdmoore
  2. In the first implementation, the skeleton loader only appeared when updating the cart after the initial load. Can we leave it without a loader in this case as well?

Jul-25-2024 14-03-09

pierorocca commented 1 month ago

If it's smooth we can follow the same approach.

On refresh, similarly if we can keep it super smooth and prevent screen shifting, then I'm ok removing the loader.

@nikkivias ?

gpressutto5 commented 1 month ago

Without a skeleton loader it just updates the text.

Aug-15-2024 12-30-48

pierorocca commented 1 month ago

Looks pretty seamless to me and updates in the same way as tax, totals, etc. do. so it's all consistent.