Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACSS: Refactor unsupported deferred intent in the blocks checkout #3866

Open
wants to merge 40 commits into
base: develop
Choose a base branch
from

Conversation

ricardo
Copy link
Member

@ricardo ricardo commented Feb 11, 2025

Fixes #3851

This PR is complementary to #3805.

Changes proposed in this Pull Request:

  • Add the required changes for mounting non-deferred intent payment methods in the blocks checkout.
  • Fix minor issue with fonts in the blocks checkout.
  • Implement loading state for non-deferred intent payment methods while the PI is being created.

Testing instructions

Follow testing instructions from #3805, but using the blocks checkout.


  • Covered with tests (or have a good reason not to test in description ☝️)
  • Added changelog entry in both changelog.txt and readme.txt (or does not apply)
  • Tested on mobile (or does not apply)

Post merge

ricardo and others added 30 commits January 22, 2025 15:40
…merce/woocommerce-gateway-stripe into fix/3804-refactor-deferred-intent
@ricardo ricardo changed the base branch from develop to fix/3804-refactor-deferred-intent February 11, 2025 05:00
appearance,
paymentMethodCreation: 'manual',
fonts: getFontRulesFromPage(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we're also fixing a missing fonts prop which resulted in a minor style issue:

Before:
no-fonts

After:
fonts

@ricardo ricardo marked this pull request as ready for review February 11, 2025 14:17
@ricardo
Copy link
Member Author

ricardo commented Feb 11, 2025

Assigned @cesarcosta99 and @wjrosa as reviewers since this PR is a follow up from #3805.

Copy link
Contributor

@wjrosa wjrosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is good and works as expected! I was able to place an order following the test steps.

One bug I noticed (maybe it is something related to my environment only) is that if I don't provide a phone number I get the following error:

Screenshot 2025-02-11 at 15 49 21

If it is something happening to everyone, we need to make the phone number required for this method.

Base automatically changed from fix/3804-refactor-deferred-intent to develop February 13, 2025 10:02
Copy link
Contributor

@cesarcosta99 cesarcosta99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and tests well 🎉 I spotted some issues, though:

  1. I experienced the same as @wjrosa and get the error Please provide complete payment details. when I don't provide a phone number. I tested this in both classic and Blocks checkouts and both errored. We'll likely need to enforce a phone requirement in the forms, maybe not something to address in this PR, but at least something to add to ACSS: Handle Errors and Edge Cases #3830.
  2. When trying to checkout with ACSS in a clean session (e.g. incognito mode), the payment method doesn't display as I fill out the form. Instead, I need to refresh the page to be able to see and select the method. I only experienced this in Blocks checkout:
output.mp4

try {
const response = await api.createIntent(
getBlocksConfiguration()?.orderId,
props.paymentMethodId
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is paymentMethodId passed only when deferred intent is not supported? If not, I think it'd look more consistent to destructure it as we're doing with other props.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACSS: Refactor unsupported deferred intent in the blocks checkout
3 participants