Automattic / woocommerce-payments

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

Many console errors related to PMME and Express Checkout Buttons are being thrown #9311

Closed pierorocca closed 2 months ago

pierorocca commented 2 months ago

Describe the bug

With WooPay, PRBs, and BNPL enabled image

Without BNPL PMME image

cc @c-shultz @FangedParakeet for heightened awareness. I am seeing some slowdown in the PMME on some sites. Not sure if there's a connection but erring on the side of caution.

pierorocca commented 2 months ago

Image

pierorocca commented 2 months ago

MIght be a mix of errors and warnings from both Fractal and Heisenberg implementation @bborman22 @FangedParakeet ?

FangedParakeet commented 2 months ago

Appearance API

These errors look like warnings produced by Stripe's Appearance API that we use to dynamically customise payment and payment messaging elements. It sounds like somewhere along the way our code is failing to ignore undefined styling properties parsed from the page and is just sending "px" (as opposed to something like "12px") to the API, warranting the warning.

AFAIA, sending malformed values to the Appearance API doesn't break any functionality and doesn't even prevent valid styles present within the same object from being applied, but would still be nice to clean up these warnings.

FangedParakeet commented 2 months ago

Please add your planning poker estimate with Zenhub @gpressutto5

gpressutto5 commented 2 months ago

The issue with invalid property for .Link and .Heading was introduced by the WooPay theming project. We discussed in this thread p1725473032776129/1725472660.619249-slack-C02CCT3F1QA and Leonardo created a PR fixing that #9385. I added a filter for valid properties for each class in #9384 following the table in the docs. This ensures we will never send invalid properties/classes to Stripe, even when they are manually added to the appearance object. However, I noticed we were already using only valid properties by checking it before creating the object here. @FangedParakeet Do you think we can discard my PR after Leonardo's PR is merged?


I couldn't replicate the px error. Even using border-radius: px !important;, getComputedStyle will just ignore it as it is invalid. @pierorocca Which theme are you using? Do you have any custom css?

leonardola commented 2 months ago

@gpressutto5 and @pierorocca Removing the class without any errors could create unwanted behavior in case the developer doesn't notice that the class is missing somewhere it should be. So IMO not adding invalid classes in the first place and letting errors show up as soon as possible in case someone does add them back is the best approach here.

pierorocca commented 2 months ago

I couldn't replicate the px error. Even using border-radius: px !important;, getComputedStyle will just ignore it as it is invalid. @pierorocca Which theme are you using? Do you have any custom css?

@gpressutto5 I don't see it anymore either. It may have been related to the issue where the button pixel size for large buttons was being set to 55.4px instead of 55px? https://github.com/Automattic/woocommerce-payments/pull/9327

gpressutto5 commented 2 months ago

@pierorocca It was not related to the 55.4px issue, but while checking that, I noticed we now have a border-radius slider, which is the cause of the problem. Thanks for helping me with that! I created #9327 to solve that problem and this PR, together with Leonardo's PR, will close this issue.