kamalbuilds / Lets-go-shop

A platform built for programming related books sale
https://letsgoshop.vercel.app/
MIT License
7 stars 17 forks source link

feat: :boom: Added more items to the in respect to issue #6 #23

Closed Omafovbe closed 2 years ago

Omafovbe commented 2 years ago

Description Added images and data for more books and updated the cart function to source images from public directory. Also, dynamically add the items to the home component

This PR fixes #6

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
letsgoshop ✅ Ready (Inspect) Visit Preview Aug 21, 2022 at 3:55PM (UTC)
Omafovbe commented 2 years ago

Thanks for the comments.

  1. The imgurl for now are the filenames of the image stored in the public folder and are referenced from home and cart component using JavaScript literal as thus .. images/${book.imgurl} which can be easily updated show Incase we need to pull the image from a remote url in the future

  2. For the basketReducer.js firstly we need to get all of the items in the JSON file and that is why I made a find call to the item database with the action.payload arg. Apart from that every other code is consistent.

Thanks qgain

On Sat, 20 Aug 2022, 05:48 Kamal Nayan, @.***> wrote:

@.**** approved this pull request.

The changes have been very Impressive 🥇 just to look at these minor details.

In src/data/books.json https://github.com/legendarykamal/Lets-go-shop/pull/23#discussion_r950643130 :

@@ -0,0 +1,202 @@

+[

I appreciate this moving to a new .json file helps in simplifing 💯

In src/data/books.json https://github.com/legendarykamal/Lets-go-shop/pull/23#discussion_r950647819 :

  • },
  • {

  • "name": "JavaScript",

  • "tagName": "javascript",

  • "price": 15,

  • "imgUrl": "javascript.jpg",

  • "numbers": 0,

  • "inCart": false

  • },

  • {

  • "name": "jQuery",

  • "tagName": "jquery",

  • "imgUrl": "jquery.png",

how are these imgurl working ? without giving full http:.... url?

In src/reducers/basketReducer.js https://github.com/legendarykamal/Lets-go-shop/pull/23#discussion_r950648596 :

 switch(action.type) {
     case ADD_PRODUCT_BASKET:
  • productSelected = { ...state.products[action.payload]}

  • // productSelected = { ...state.products[action.payload] }

  • productSelected = { ...state.books.find(book => book.tagName === action.payload) };

This is wondurful to add the new logic but At all other places we are using action.payload as arg , just for consistency comment the new logic and uncomment the older one .

— Reply to this email directly, view it on GitHub https://github.com/legendarykamal/Lets-go-shop/pull/23#pullrequestreview-1079463944, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXP2H7SZY2XKGC7YFPCJUDV2BPTBANCNFSM57BXT3VQ . You are receiving this because you authored the thread.Message ID: @.***>

kamalbuilds commented 2 years ago

thanks @Omafovbe these details would be helpful to be put up in README.

kamalbuilds commented 2 years ago

Hey @Omafovbe were these changes merged to the main branch :thinking: I dont see them

Omafovbe commented 2 years ago

You didn't merge to the main branch but merge to a new branch in the repo. Check

On Sat, 3 Sept 2022, 08:29 Kamal Nayan, @.***> wrote:

Assigned #23 https://github.com/legendarykamal/Lets-go-shop/pull/23 to @Omafovbe https://github.com/Omafovbe.

— Reply to this email directly, view it on GitHub https://github.com/legendarykamal/Lets-go-shop/pull/23#event-7316725710, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXP2H6FDO65CKNBYIAOEYTV4L45FANCNFSM57BXT3VQ . You are receiving this because you were assigned.Message ID: @.***>

kamalbuilds commented 2 years ago

@Omafovbe I dont see it there too https://github.com/legendarykamal/Lets-go-shop/compare/new...master

Better please open a new PR for merging on the master branch .

kamalbuilds commented 2 years ago

I am eager to see your name in the contribitors list :smile:

kamalbuilds commented 2 years ago

also @Omafovbe revert your last commit done on aug 21 https://github.com/legendarykamal/Lets-go-shop/compare/master...Omafovbe:Lets-go-shop:add-more-items by this command on the add to item branch

git reset --soft HEAD~1

and then open a PR against the master branch

as I dont know why you have merged your master branch to ur add to item branch only as per this https://github.com/legendarykamal/Lets-go-shop/compare/master...Omafovbe:Lets-go-shop:add-more-items

Omafovbe commented 2 years ago

Done, reset the branch and opened a new PR. Thanks for the opportunity

On Sat, Sep 3, 2022 at 9:12 AM Kamal Nayan @.***> wrote:

also @Omafovbe https://github.com/Omafovbe revert your last commit done on aug 21 master...Omafovbe:Lets-go-shop:add-more-items https://github.com/legendarykamal/Lets-go-shop/compare/master...Omafovbe:Lets-go-shop:add-more-items by this command on the add to item branch

git reset --soft HEAD~1

and then open a PR against the master branch

as I dont know why you have merged your master branch to ur add to item branch only as per this master...Omafovbe:Lets-go-shop:add-more-items https://github.com/legendarykamal/Lets-go-shop/compare/master...Omafovbe:Lets-go-shop:add-more-items

— Reply to this email directly, view it on GitHub https://github.com/legendarykamal/Lets-go-shop/pull/23#issuecomment-1236073004, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXP2HYUDFC7BAKIOZUTWYDV4MB75ANCNFSM57BXT3VQ . You are receiving this because you were mentioned.Message ID: @.***>