This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Display shipping calculator link for guests shopper #11442
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Earlier, shipping calculator was getting displayed based on the isAddressComplete value. - For the scenarios where address was incomplete but formatted address was present shipping calculator was not getting displayed. - This made shipping calculator not getting displayed for guest shoppers. - With this, conditions are changed from isAddressComplete to formatShippingAddress to display shipping calculator if formatted address is present. - Add unit test case for the condition for formatted address.
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
assets/js/base/components/cart-checkout/totals/shipping/test/index.tsx
|
|
Size Change: +41 B (0%) Total Size: 1.53 MB
ℹ️ View Unchanged
|
tarunvijwani
changed the title
woocommercebot
requested review from
a team and
senadir
and removed request for
a team
tarunvijwani
added
status: needs review
block: cart
tarunvijwani
requested review from
rubikuserbot and
mikejolley
and removed request for
rubikuserbot
|
@tarunvijwani to be super safe add also testing steps where the shipping address has all fields empty and the Add and address link to see shipping options link appears. That would imply not defaulting shipping to the store address |
ralucaStan
reviewed
Oct 25, 2023
| const hasFormattedAddress = !! formatShippingAddress( shippingAddress ); | ||
|
|
||
| // If we don't have formatted address, and we're not in the editor, don't show anything. | ||
| if ( ! hasFormattedAddress && ! isEditor ) { |
There was a problem hiding this comment.
This comments is redundant as it explains the code, but does not provide context. What case are we actually covering with this and why are we returning nothing?
There was a problem hiding this comment.
Yup, maybe the comment can explain why hasFormattedAddress would be false? (i.e. the shopper hasn't entered one)
There was a problem hiding this comment.
@ralucaStan Thank you for the suggestion, I will update the comment and add more testing steps.
What case are we actually covering with this and why are we returning nothing?
If there is no default customer location in the store and the customer has not entered their address, but there is a default shipping method available for all locations, then we will hide the shipping calculator. This is because the store already has a default shipping address. This change was made in response to this request: #8141 (comment)
Here are the steps to reproduce this behavior:
- Go to the WooCommerce settings page:
wp-admin/admin.php?page=wc-settings. - Change 'Default customer location' to
No location by default. - Add any default(fallback) shipping rates.
- Open your site in a private session(incognito mode).
- Add some products to the cart.
- Go to the cart page, and confirm the shipping calculator is hidden and default shipping rates are visible
There was a problem hiding this comment.
I understand that we have this rather complex sutation because we choose to have two buttons, one named "change address" and one named "enter an address to view shipping rates"? Otherwise I don't understand why we need to check if a formatted address is present to begin with to see the calculator.
There was a problem hiding this comment.
@senadir Yes, it wouldn't make sense to show the 'Add address for shipping rates' link when default rates are already displayed. Similarly, we should not display the 'Change address' link without an address. Hence we decided to hide it in this condition.
There was a problem hiding this comment.
What happens when you have rates but not an address? Do we show nothing at that step?
There was a problem hiding this comment.
@senadir No, we don't show a calculator in that scenario. It will be addressed in the https://github.com/woocommerce/woocommerce-blocks/issues/10327 ticket.
opr
approved these changes
Oct 25, 2023
There was a problem hiding this comment.
Changes look fine besides the comment Raluca mentioned, a couple of the testing steps seem slightly redundant, why make the tester add rates for California, then update their store's address? We could just specify that there should be rates that cover the store's address couldn't we?
I tested with:
- above steps
- no shipping rates for the default location
- only local pickup for the default location
Since this works OK and the updates are only comments/testing steps I will approve.
| const hasFormattedAddress = !! formatShippingAddress( shippingAddress ); | ||
|
|
||
| // If we don't have formatted address, and we're not in the editor, don't show anything. | ||
| if ( ! hasFormattedAddress && ! isEditor ) { |
There was a problem hiding this comment.
Yup, maybe the comment can explain why hasFormattedAddress would be false? (i.e. the shopper hasn't entered one)
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
- Update the comments to add the reason to hide shipping calculator. - When there is no default customer location in the store and the customer has not entered their address, but there is a default shipping method available for all locations, then we will hide the shipping calculator.
|
@opr I have updated the testing steps and comments. I'd appreciate it if you could review them before I merge the PR. Thank you so much! |
opr
suggested changes
Oct 25, 2023
@opr There is a discussion ongoing about which text to show for the calculator link. Click here #11442 (comment) for more information. I feel we should tackle that in a different PR. Happy to hear your thoughts about it. |
|
@tarunvijwani - ok if you create a follow-up issue to tackle this then that should be OK! |
opr
approved these changes
Oct 25, 2023
We already have a separate ticket for it. It will be addressed in the woocommerce/woocommerce#42280 ticket. |
tarunvijwani
deleted the
fix/11435-display-shipping-calculator-for-guests
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Fixes #11435
Why
isAddressCompletevalue.isAddressCompleteatoformatShippingAddressto display the shipping calculator if the formatted address is present.Testing Instructions
Case 1
wp-admin/admin.php?page=wc-settings.No location by default.Case 2
wp-admin/admin.php?page=wc-settings.No location by default.Case 3
wp-admin/admin.php?page=wc-settings.Shop country/region.Screenshots or screencast
WooCommerce Visibility
Required:
Checklist
Required:
[type]label or a[skip-changelog]label.Conditional:
[skip-changelog]label is not present).Changelog