-
Notifications
You must be signed in to change notification settings - Fork 219
Move StoreNoticesContainer
to @woocommerce/blocks-checkout
package and add tests
#7558
Conversation
The release ZIP for this PR is accessible via:
|
Script Dependencies ReportThe
This comment was automatically generated by the |
Size Change: -20.8 kB (-2%) Total Size: 970 kB
ℹ️ View Unchanged
|
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. |
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.
Nice on, everything looks good, just a non-blocking comment - see below 👍
StoreNoticesContainer, | ||
SnackbarNoticesContainer, | ||
} from '@woocommerce/base-context'; | ||
import { SnackbarNoticesContainer } from '@woocommerce/base-context'; |
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.
Do we want to move the SnackbarNoticesContainer
too?
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.
Yes, I'll do it in another PR next cooldown. #7638 open to track. Thanks for pointing it out Alex.
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 no we don't want to do it now because as Mike said, the SnackbarNoticesContainer
isn't location dependent, so third parties can just add a notice with type snackbar
and add it to either the cart
or checkout
contexts depending where the notice occurs.
5773d01
to
0091e9d
Compare
TypeScript Errors ReportFiles with errors: 431
assets/js/blocks/cart-checkout-shared/payment-methods/payment-method-error-boundary.js
packages/checkout/components/store-notices-container/index.tsx packages/checkout/components/store-notices-container/test/index.tsx |
Really nice work @opr! I think this doc should be updated as well. https://github.com/woocommerce/woocommerce-blocks/blob/4d1c295a2bace9a4f6397cfd5469db31083d477a/docs/internal-developers/block-client-apis/notices.md#storenoticescontainer? Can you think of other docs we might need to update in the extensibility section? |
Hey @ralucaStan thanks for the feedback, what other things do you think should change on that document? (besides the typo in |
622d110
to
58979dc
Compare
I missed this ping Thomas. I was thinking about the path used in the import. Should that be changed?
|
dcb41f6
to
5a1e6b9
Compare
This PR moves
StoreNoticesContainer
to@woocommerce/blocks-checkout
package. This is possible now since we moved notices to use only@wordpress/notices
'score/notices
data store. Previously we had some hooks and contexts which meant this component could not be exported from blocks, since the Context instances would be different.Testing
Automated Tests
User Facing Testing
4000 0000 0000 9995
ensure the error appears in the payment methods area.WooCommerce Visibility
Performance Impact
Changelog