-
Notifications
You must be signed in to change notification settings - Fork 378
ShopPayButton now accepts storeDomain prop #645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
|
|
||
| const ShopifyContext = createContext<ShopifyContextValue>({ | ||
| export const defaultShopifyContext: ShopifyContextValue = { | ||
| storeDomain: 'test', |
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.
Noticed that this makes all uses of useShop() to contain this default parameters. I would suggest to remove this and make it null by default. This is why in the step above I had to check if storeDomain was the default or not.
|
Added @juanpprieto in a commit since I copied some of this code from: Shopify/hydrogen-react#122 |
d59f285 to
46080f9
Compare
|
Working oxygen deployment at: https://bwcgr1r9v-f1f4fa724b7467f41f07.myshopify.dev/ |
Co-authored-by: Juan P. Prieto <juanpablo.prieto@shopify.com>
This reverts commit 6023e83.
bc1563e to
62c5df0
Compare
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.
I updated docs
I think the default context is there is so that ShopPayButton doesn't completely breaks when props not supplied.
ShopPayButton can now receive a storeDomain prop. Updated the demo store to make it work.
WHY are these changes introduced?
Part of: #629
Solves:
#487
HOW to test your changes?
Go to a product in the demo store and click on Shop Pay button. It should open the Shop checkout
Post-merge steps
There are other component relying on
ShopifyProviderChecklist