This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Add validation error to prevent checkout when there is no shipping method available #8384
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
|
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
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: +298 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
senadir
reviewed
Feb 6, 2023
assets/js/base/components/cart-checkout/totals/shipping/index.tsx
Outdated
Show resolved
Hide resolved
senadir
reviewed
Feb 6, 2023
assets/js/base/components/cart-checkout/shipping-rates-control-package/package-rates.tsx
Outdated
Show resolved
Hide resolved
senadir
suggested changes
Feb 6, 2023
There was a problem hiding this comment.
This is not working for me :/
This is now working for me, but it still seems to be buggy.
If you go regular shipping, place, get an error, switch to local pickup, the error goes away, switch back to regular shipping, the error surfaces again for some seconds before it disappears.
senadir
suggested changes
Feb 7, 2023
There was a problem hiding this comment.
Hey @tarunvijwani , we're going to put this on hold until @elizaan36 figures out some design next steps when it comes to validation and text.
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
|
When the user first lands on the Checkout block with no address provided, can we hide the "Shipping options" section? Then if an address is entered that's not within a valid shipping zone, we can present a notice inline in the shipping options section. The top level notice isn't necessary in this case. Let me know what you think @tarunvijwani @senadir
|
|
Hey @elizaan36 this will still get us to the same place of needing something to prevent the customer from placing order. The issue here is that a shopper still has Local Pickup selected (even if not visible) and they can place the order with the error shown. |
|
Noting that we don't have information-type notices implemented in the project. We could display a normal error notice in the shipping section until we have the new notice types added in the blocks. Does that sound good to you @elizaan36 ? |
|
Yep, sounds good. Based on the conversation in Slack, there should be the same pattern/message for the same error whether local pickup is enabled or not. When pickup is not enabled, there's an error notice in the Shipping options section.
Then if user proceeds to checkout there's a top level notice.
|
|
Thank you, @senadir and @elizaan36, for the review. I have updated the code. 👍 |
mikejolley
suggested changes
Feb 27, 2023
assets/js/base/context/providers/cart-checkout/checkout-processor.ts
Outdated
Show resolved
Hide resolved
assets/js/blocks/checkout/inner-blocks/checkout-shipping-method-block/block.tsx
Outdated
Show resolved
Hide resolved
tarunvijwani
commented
Feb 27, 2023
| @@ -124,7 +124,7 @@ const processInvalidParamResponse = ( | |||
| */ | |||
| export const processErrorResponse = ( | |||
| response: ApiErrorResponse, | |||
| context: string | undefined | |||
| context?: string | undefined | |||
There was a problem hiding this comment.
This change will fix the TypeScript error present in the assets/js/base/context/providers/cart-checkout/checkout-processor.ts file for not passing two arguments.
|
@mikejolley Thank you for the review. I have updated the code. 👍 |
|
@tarunvijwani this needs a rebase - ready for final review? |
added 2 commits
March 16, 2023 11:06
- Add the validation error in ShippingSelector to prevent checkout when no shipping rates available. - Add notice to Checkout processor so that notice appears after clicking on place order button.
- Make multiple prop as optional in RatePrice component to fix TypeScript error.
8932bc9 to
d77e28b
Compare
tarunvijwani
commented
Mar 16, 2023
| @@ -16,7 +16,7 @@ export const RatePrice = ( { | |||
| }: { | |||
| minRate: CartShippingPackageShippingRate | undefined; | |||
| maxRate: CartShippingPackageShippingRate | undefined; | |||
| multiple: boolean; | |||
| multiple?: boolean; | |||
There was a problem hiding this comment.
This change will fix the TypeScript error in assets/js/blocks/checkout/inner-blocks/checkout-shipping-method-block/block.tsx for not passing the multiple prop.
@mikejolley I have fixed the conflicts. This is now ready for review. Thank you! 🙂 |
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.
With this PR, we're preventing checkout if no shipping methods are available.
Fixes #8348
Changes in the PR:
Accessibility
prefers-reduced-motionOther Checks
Screenshots
Testing
Automated Tests
User Facing Testing
WooCommerce Visibility
Performance Impact
Changelog