Refresh page when ECE is dismissed #9888
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. |
rafaelzaleski
changed the title
|
Size Change: +48.3 kB (+4%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
|
Size Change: +1.21 kB (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
rafaelzaleski
requested review from
asumaran and
frosso
and removed request for
a team
There was a problem hiding this comment.
@rafaelzaleski @pierorocca when using an address from the UK or Canada, the refresh doesn’t work correctly. It seems that for addresses in those countries, shipping methods cannot be calculated because the full zip code cannot be retrieved. It might be better to disable this new behavior for the UK, Canada, and any country where shipping options fail to load with the data provided by the payment sheet.
I also wanted to point out that the billing address is NOT being updated. If the customer has a previously saved address in the session, it remains unchanged even after updating the shipping address. While this might not be an issue, I thought it was worth mentioning to keep in mind.
| if ( ! [ 'cart', 'checkout' ].includes( context ) || ! lastSelectedAddress ) | ||
| return; |
|
@asumaran, I'm now bypassing If the user refreshes the page, they’ll see the incorrect (redacted) address. The issue is that the Blocks API won't accept a redacted postcode for calculating shipping options, which is why a live refresh isn’t possible for CA and UK addresses. |
|
Asked on the Stripe slack channel if they know why the address is redacted and if that's expected. It's odd it only applied to some countries. |
There was a problem hiding this comment.
@rafaelzaleski I found a small discrepancy between the shortcode and block implementations. If the address is changed in the cart block page, it is correctly reflected on the cart page in both the block and shortcode pages.
However, on the shortcode Cart page, if we reopen the payment sheet and select the same address, the address changes once the dialog is closed. The state/county is removed. See the video for reference.
Screen.Recording.2024-12-16.at.7.38.38.PM.mov
So far, this behavior seems to occur with a 🇵🇪 Peruvian address only. Addresses in Brazil and the US appear to work correctly.
This can lead to unexpected behaviors** if we have specific rates for given city/county as it will appears the shipping options changes.
Here’s another video where the issue can be seen. In the payment sheet, the shipping rate shows $60, but after closing the payment sheet, the shipping rate changes to $5, which is the default rate for “Rest of the world” It should remain $60, which is the specific rate for “Municipalidad Metropolitana de Lima.”
Screen.Recording.2024-12-16.at.8.01.23.PM.mov
I think it’s worth looking into what’s going on.
Other than that I left a couple of small comments for you so far.
I haven't finished testing all the cases I wanted. I'll continue later.
| if ( | ||
| [ 'cart', 'checkout' ].includes( context ) && | ||
| lastSelectedAddress && | ||
| ! [ 'CA', 'GB' ].includes( lastSelectedAddress.country ) |
There was a problem hiding this comment.
Can we move this array, along with the accompanying comment, into a self-explanatory variable, either within this function or at a higher level? As this list will likely need to be updated with more countries in the future, using a variable name and relocating it would improve readability and maintainability.
| if ( | ||
| [ 'cart', 'checkout' ].includes( context ) && | ||
| lastSelectedAddress && | ||
| ! [ 'CA', 'GB' ].includes( lastSelectedAddress.country ) |
There was a problem hiding this comment.
Same here. Let's put this array in a variable.
There was a problem hiding this comment.
Since this mirrors the code in the other file, I’ll wait until we complete the review process before copying the changes over to tokenized carts. You can focus your review on the code outside tokenized carts for now.
| /** | ||
| * External dependencies | ||
| */ |
There was a problem hiding this comment.
Just curious: is it necessary to include this “external dependencies” comment even if we’re not importing a single module?
@pierorocca, it looks like this is how Google Pay was implemented (reference). I don’t think Stripe has control over this behavior. I believe the redacted version (showing only the first few digits) is sufficient to calculate shipping costs for a specific region, which is the purpose of the intermediate address. However, proper support for calculating rates with partial postcodes would need to be added in Woo Core. Right now, we’re just hacking a workaround for this. |
@asumaran, Peruvian states are coming through the payment sheet as full text instead of the abbreviated 3-letter code. I added a fallback to use the option text when the option value doesn’t match the payment sheet here: 08d18fc |
There was a problem hiding this comment.
Alright, I’ve tested this on the product, cart and checkout pages (both shortcode and block-based), and everything is working as expected.
The Peruvian address issue has been resolved.
However, the issue with addresses from countries affected by the zip code limitation still persists. Unfortunately, there’s not much we can do about it but the address isn’t refreshed as expected in those cases.
That said, I think the PR is good to go. Just make sure to apply the changes to the Tokenized Carts implementation. I’ve tested the happy path with the current changes, and it’s working fine there.
| * @return {boolean} True if the postcode is redacted for the country, false otherwise. | ||
| */ | ||
| const isPostcodeRedactedForCountry = ( country ) => { | ||
| return country.length === 2 && [ 'CA', 'GB' ].includes( country ); |
There was a problem hiding this comment.
Just curious, why are we checking the country name length here? Wouldn’t the .includes method be sufficient on its own?
There was a problem hiding this comment.
It was initially to ensure we're receiving an alpha-2 ISO code, but you’re right—it's redundant with the array check. I've removed it here.
I'm a little worried that there's so much information missing if the shopper exits and checks out using the form. It's more likely than not that the shopper will submit and encounter validation errors. My question is how is Shopify able to do that with the same Google Pay integration? I'm asking Stripe anyway in case this happens to be a gated feature. |
Fixes #9794
Changes proposed in this Pull Request
Refresh the page's shipping information when ECE is dismissed. This ensures that the shipping options on the cart and checkout pages are updated without requiring the user to reload the page.
Testing Instructions
Repeat the following steps for:
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