-
Notifications
You must be signed in to change notification settings - Fork 221
Move Button, StoreNotice and StoreNoticesContainer components into the components package #11766
Conversation
…e components package
The release ZIP for this PR is accessible via:
Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
packages/components/store-notices-container/index.tsx
packages/components/store-notices-container/snackbar-notices.tsx packages/components/store-notices-container/test/index.tsx |
861d561
to
1fc4b9d
Compare
Size Change: +18.9 kB (+1%) Total Size: 1.56 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.
Hey @alexflorisca couple of comments on this one:
- Can we find existing uses of
Button
,StoreNotice
, andStoreNoticesContainer
and ensure they're being imported from@woocommerce/blocks-components
- I know ultimately they will come from the same place regardless, but to make reasoning with the code easier for posterity I think it would be good to update the imports. - The testing steps are a good start, but better would be to find uses of each of these components and figure out how we can test that they're importing correctly. e.g. for the store notice containers I would add something to
core/notices
with thewc/cart
context and make sure it shows up on the cart page.
Thanks for your comments @opr, I've updated the references to Can you take another look? |
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.
Yep looks good now, just testing one instance of the store notices should be fine! There are some conflicts now so if you rebase I'll re-test!
Done. |
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.
Looks good now, tested and it's working well.
What
This PR moves the Button, StoreNotice and StoreNoticesContainer components into the components package
Fixes #11685
Fixes #11683
Why
We are consolidating all the components that we will export for external use into a
components
packageTesting Instructions
Please consider any edge cases this change may have, and also other areas of the product this may impact.
Internal Developer Testing (do not include in the notes)
let { createErrorNotice } = wp.data.dispatch('core/notices'); createErrorNotice('Hello there', { context: 'wc/cart' });
WooCommerce Visibility
Required:
Checklist
Required:
[type]
label or a[skip-changelog]
label.Conditional:
[skip-changelog]
label is not present).Changelog