search

notrab / react-use-cart

React hook library for managing cart state
http://npm.im/react-use-cart
Apache License 2.0
367 stars 48 forks source link

fix: setItems type fixed in CartProviderState #84

Closed Skidragon closed 3 years ago

Skidragon commented 3 years ago

The reason why I made a change to the setItems function is because I was getting an NaN for totalItems.

notrab commented 3 years ago

Thanks for the PR @Skidragon!

Are you happy for me mark this ready for review?

Skidragon commented 3 years ago

@notrab yep!

Skidragon commented 3 years ago

@notrab Should we implicitly add a quantity of 1 to each item in setItems? I noticed in defaultItems doesn't have a default quantity added to the item.

notrab commented 3 years ago

Sounds good! I don’t think that would cause any backwards compatibility issues so feel free to add it to this PR and I’ll get it merged 🔥

Skidragon commented 3 years ago

@notrab I made the update to the documentation, I think the CartProvider defaultItems may need to have a quantity of 1 implicitly set also for consitancy and the docs doesn't talk about it either but I believe it should be for a separate PR since this is for setItems only and don't want to mix concerns?

notrab commented 3 years ago

@Skidragon sure.

Is this PR ready for review now? 😄

Skidragon commented 3 years ago

@Skidragon sure.

Is this PR ready for review now? 😄

Lol yeah. I got nervous and just made sure the tests covered everything.

Skidragon commented 3 years ago

@notrab the small things have been added with tests still passing :)

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 1.11.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: