TITAN-315 - empty state cartTable + cartQuickView #35
Conversation
pages/cart.js
Outdated
| import CartTable from '../components/Cart/CartTable'; | ||
| import CartTotals from '../components/Cart/CartTotals'; | ||
| import empty from "../data/stubs/cart/empty"; | ||
| import single from "../data/stubs/cart/single"; |
There was a problem hiding this comment.
single isn't being used here so we can comment it out while testing empty
pages/cart.js
Outdated
| </> | ||
| )} | ||
| {isCartEmpty && !isCartLoading && <p>You have no items in cart</p>} | ||
| {isCartLoading && <Loader />} |
There was a problem hiding this comment.
Loader is missing an import
There was a problem hiding this comment.
I'm taking care of it in my PR
There was a problem hiding this comment.
good call, lets remove it
components/Cart/CartTable.tsx
Outdated
| @@ -0,0 +1,30 @@ | |||
| import React from "react" | |||
| import useAtlasShopify from "../../hooks/useAtlasShopify"; | |||
| import { AiOutlineCloseCircle} from "react-icons/ai" | |||
There was a problem hiding this comment.
These aren't being used. If you run npm run lint you should get warnings about unused imports as you are working
components/Header/CartQuickView.js
Outdated
| return acc + item.quantity; | ||
| }, 0); | ||
| } | ||
| const cart = empty.cart; |
There was a problem hiding this comment.
I'm tackling this part in my PR as well
There was a problem hiding this comment.
Please:
- Add a description to the PR, highlighting why some decisions were made
- Add steps on how to test this PR
- Be mindful of blank lines missing from the end of some files, I suggestion I have is to use GitHub's own Pull Request UI. Before creating the PR, make sure to scroll down and scan all the files to be included, you will notice a crossed red circle when the line is missing
- The code duplication is a code smell, it adds risk to the maintainability of the Cart page and quick view. Suggestion:
CartServiceor an adaptable component
| const cartItems = cart.lines.nodes; | ||
| const cartCount = cartItems.length | ||
| const isCartEmpty = cartCount === 0; | ||
| const isCartLoading = false; | ||
| const cartSubTotal = cart.cost.subtotalAmount.amount; | ||
| const cartTotal = cart.cost.totalAmount.amount; | ||
| const checkoutUrl = cart.checkoutUrl; |
There was a problem hiding this comment.
Five of these seven lines are duplicated in CartQuickView.js we should address this, maybe creating a Cart service? Any suggestions?
There was a problem hiding this comment.
I'm already tackling this, but I'm blocked by changes in this PR
alvarocavalcanti
changed the title
There was a problem hiding this comment.
LGTM! Nice job addressing the comments. We can tackle the other bits in @mikepawlinski PR
|
@kleczynski Please try to use descriptive commit messages instead of Think that once this code gets merged and someone is reading through the commit history trying to figure out something they won't have clarity on what was added on that commit. |
Description
My task was to reflect the components of the store card based on the bigcommerce blueprint. I dealt with the card section as well as a quick preview of what's in it from the home page. Based on the mock data, I had to write a logical equivalent for an empty shopping cart. In addition, slight changes were made to the UI for styling ( example: Adjusting the font of the text informing about the empty card, etc )
https://wpengine.atlassian.net/browse/titan-315
Testing
Basically, the site is set up to test the lack of items in the card, just like other stores where we have nothing in the shopping cart when we first enter. Basically, the site is set up to test the lack of items in the card, just like other stores where we have nothing in the shopping cart when we first enter. If there would be visible items in the card for some reason then in the
useCart.jsfile change constCART_STATE_KEY = "single/multiple";to any text that ( e.g. "cart-state")Screenshots
Documentation Changes
Dependent PRs