Skip to content
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

Ecwid Provider, Shopping Bag, Product Browser views parameter #8

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Christopotamus
Copy link

Hello!

I'm not sure if you're open to pull requests or if I should make an issue and link to the branch first. Let me know if I should open an issue instead.

This pull request includes changes that:

  1. Adds the shopping cart component
  2. Moves all of the Ecwid loading into a React Provider
  3. Adds view props to ProductBrowser for some light customization

Along with moving Ecwid loading into the Provider, I rewrote the initialization of the three components to wait for API Ready (via Provider state variable) and then call their respective initialization functions. This seems to work well in my testing so far, but I've only tried with app router.

My gut also tells me that the document.body.appendChild should be removed and the whole process of injecting ecwid's script needs to be made more react/next friendly, but this is what I was able to get to so far and has solved my immediate problem of needing store + cart.

I've also updated the README.md file to include instructions on how to use it via Wrapping the included EcwidProvider component in a client component, and then wrapping your store page in that. This works with app router for me, but I haven't tested with pages router.

Chris Budd added 3 commits February 12, 2024 16:25
Added ShoppingCart Component which has exposed data properties
Added EcwidProvider and EcwidContext to hold state of Ecwid script
loading

Rewrote initialization of all components to initialize based on Provider
having loaded Ecwid and API being ready.
@meteor-ec
Copy link
Contributor

Hi @Christopotamus,

Thanks for your PR! Great job!

We'll check it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants