-
Notifications
You must be signed in to change notification settings - Fork 219
Move notices to corresponding context #8228
Conversation
… .editorconfig file enforces 2 spaces.
Jobs renaming.
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: +97 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexflorisca During our call yesterday, you suggested changing context
to errorMessageContext
to increase the readability. I noticed that createErrorNotice()
expects that parameter to be called context
. To use errorMessageContext
instead, we'd have to apply the following changes:
Change
woocommerce-blocks/assets/js/base/context/providers/cart-checkout/checkout-processor.ts
Line 170 in d374528
context: 'wc/checkout/payments', |
errorMessageContext: 'wc/checkout/payments',
Change
woocommerce-blocks/assets/js/base/context/providers/cart-checkout/checkout-processor.ts
Line 179 in d374528
context: 'wc/checkout/shipping-methods', |
errorMessageContext
Change
woocommerce-blocks/assets/js/data/checkout/thunks.ts
Lines 73 to 80 in d374528
( { | |
errorMessage, | |
validationErrors, | |
context = 'wc/checkout', | |
} ) => { | |
createErrorNotice( errorMessage, { context } ); | |
setValidationErrors( validationErrors ); | |
} |
response.forEach(
( {
errorMessage,
validationErrors,
errorMessageCcontext = 'wc/checkout',
} ) => {
createErrorNotice( errorMessage, {
context: errorMessageContext,
} );
setValidationErrors( validationErrors );
}
);
With that in mind, I find the current implementation easier to read, but I'm keen to hearing your thoughts on that.
…f github.com:woocommerce/woocommerce-blocks into 7731-remove-hardcoded-payment-notice # Conflicts: # .github/workflows/unit-tests.yml # package-lock.json
Yep you're right @nielslange , let's leave it as it is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working as described 👍🏼
…commerce/woocommerce-blocks into 7731-remove-hardcoded-payment-notice # Conflicts: # docs/third-party-developers/extensibility/checkout-payment-methods/payment-method-integration.md
Fixes #7731
Currently, the error message
There was a problem with your payment option.
appears above the Checkout block. In #7711, @mikejolley added new contexts forStoreNoticesContainer
. This issue introduces the following changes:There was a problem with your payment option.
fromwc/checkout
towc/checkout/payments
.There was a problem with your shipping option.
fromwc/checkout
towc/checkout/shipping-methods
.__internalEmitValidateEvent
to exceptcontext
as an optional parameter.Additional notes
As mentioned before, this PR introduces
context
as an optional parameter, which enables us (and 3PD) to define in which context an error message shall appear.Screenshots
There was a problem with your payment option.
There was a problem with your shipping option.
Testing
User Facing Testing
/assets/js/base/context/providers/cart-checkout/checkout-processor.ts
and changeto
Place Order
and verify that the error messageThere was a problem with your payment option.
appears within thePayment options
section./assets/js/base/context/providers/cart-checkout/checkout-processor.ts
and changeto
Place Order
and verify that the error messageThere was a problem with your shipping option.
appears within theShipping options
section.WooCommerce Visibility