-
Notifications
You must be signed in to change notification settings - Fork 0
redesign cart shopping #98
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
base: main
Are you sure you want to change the base?
Conversation
| backgroundColor: theme.palette.disabledCard.background, | ||
| color: theme.palette.disabledCard.text, | ||
| color: defaultTextColor, | ||
| padding: `${theme.spacing(1 / 2)} ${theme.spacing(1)}`, |
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.
there isnt a way to add color from pallete ? (following mui documentation)
| @@ -4,115 +1,132 @@ | |||
| import { Grid, Box, Divider } from '@mui/material'; | |||
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.
no use of Box and Divider, please remove
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.
Pull Request Overview
This PR redesigns the cart shopping interface with a new layout and styling approach. The changes transform the cart from a vertical layout to a modern two-column design with improved visual hierarchy and typography variants.
Key changes include:
- Introduction of new text variants for better typography consistency across cart components
- Complete restructuring of cart layout from vertical to horizontal grid-based design
- Simplified product card component by removing unused props and improving status handling
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/constants/types.js | Adds new text variant constants for cart typography and removes trailing whitespace |
| src/constants/hardText.js | Adds new cart-related text constants and status values |
| src/components/shared/Text/textVariants.js | Defines styling for new text variants with specific typography rules |
| src/components/Product/ProductCard.jsx | Simplifies component by removing unused props and improving status comparison |
| src/components/Product/Product.styles.js | Updates styling to use consistent text colors and removes unused theme parameter |
| src/components/Cart/Cart.styles.js | Complete redesign of cart styling with new grid layout and card-based design |
| src/components/Cart/Cart.jsx | Restructures cart component to use new two-column layout with improved product display |
| export const ProductImage = { | ||
| width: '150px', | ||
| height: '150px', | ||
| objectFit: 'cover', | ||
| borderRadius: '14px', | ||
| marginRight: '2rem', | ||
| }; |
Copilot
AI
Aug 6, 2025
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.
ProductImage should be a styled component function like other exports in this file, rather than a plain object. This creates inconsistency in the styling approach.
| export const ProductImage = { | |
| width: '150px', | |
| height: '150px', | |
| objectFit: 'cover', | |
| borderRadius: '14px', | |
| marginRight: '2rem', | |
| }; | |
| export const ProductImage = styled('img')(() => ({ | |
| width: '150px', | |
| height: '150px', | |
| objectFit: 'cover', | |
| borderRadius: '14px', | |
| marginRight: '2rem', | |
| })); |
| export const ProductName = styled('div')(() => ({ | ||
| fontWeight: 800, | ||
| fontSize: '1.6rem', | ||
| marginBottom: '1rem', | ||
| textTransform: 'capitalize', | ||
| })); |
Copilot
AI
Aug 6, 2025
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.
ProductName styled component duplicates styling that already exists in textVariants.js (PRODUCT_TITLE). Consider using the existing text variant instead of creating duplicate styles.
| export const ProductName = styled('div')(() => ({ | |
| fontWeight: 800, | |
| fontSize: '1.6rem', | |
| marginBottom: '1rem', | |
| textTransform: 'capitalize', | |
| })); | |
| // Use <SharedTypography variant="PRODUCT_TITLE" /> for product names instead of a custom styled component. |
| export const ProductPrice = styled('div')(() => ({ | ||
| fontWeight: 700, | ||
| fontSize: '1.4rem', | ||
| color: '#111', | ||
| marginBottom: '1rem', | ||
| })); |
Copilot
AI
Aug 6, 2025
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.
ProductPrice styled component duplicates styling that already exists in textVariants.js (PRODUCT_PRICE). Consider using the existing text variant instead of creating duplicate styles.
| export const ProductPrice = styled('div')(() => ({ | |
| fontWeight: 700, | |
| fontSize: '1.4rem', | |
| color: '#111', | |
| marginBottom: '1rem', | |
| })); | |
| export const ProductPrice = styled(SharedTypography)({ | |
| marginBottom: '1rem', | |
| }); |
| export const ProductLocation = styled('div')(() => ({ | ||
| fontSize: '1.1rem', | ||
| color: 'gray', | ||
| marginBottom: '2rem', | ||
| })); |
Copilot
AI
Aug 6, 2025
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.
ProductLocation styled component duplicates styling that already exists in textVariants.js (PRODUCT_LOCATION). Consider using the existing text variant instead of creating duplicate styles.
| export const ProductLocation = styled('div')(() => ({ | |
| fontSize: '1.1rem', | |
| color: 'gray', | |
| marginBottom: '2rem', | |
| })); | |
| export const ProductLocation = styled(SharedTypography).attrs({ | |
| variant: 'PRODUCT_LOCATION', | |
| })({ | |
| marginBottom: '2rem', | |
| }); |
| export const SummaryTitle = styled('div')(() => ({ | ||
| fontWeight: 800, | ||
| fontSize: '1.5rem', | ||
| marginBottom: '2rem', | ||
| })); |
Copilot
AI
Aug 6, 2025
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.
SummaryTitle styled component duplicates styling that already exists in textVariants.js (SECTION_TITLE). Consider using the existing text variant instead of creating duplicate styles.
| export const SummaryTitle = styled('div')(() => ({ | |
| fontWeight: 800, | |
| fontSize: '1.5rem', | |
| marginBottom: '2rem', | |
| })); | |
| // Use SharedTypography with variant="SECTION_TITLE" instead of SummaryTitle |
| export const SummaryRow = styled('div')(() => ({ | ||
| display: 'flex', | ||
| justifyContent: 'space-between', | ||
| fontSize: '1.2rem', | ||
| marginBottom: '1.2rem', | ||
| })); |
Copilot
AI
Aug 6, 2025
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.
SummaryRow styled component duplicates styling that already exists in textVariants.js (SUMMARY_ROW). Consider using the existing text variant instead of creating duplicate styles.
| export const SummaryRow = styled('div')(() => ({ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| fontSize: '1.2rem', | |
| marginBottom: '1.2rem', | |
| })); | |
| // Removed duplicate SummaryRow styled component. Use SharedTypography with variant="SUMMARY_ROW" instead. |
| export const SummaryTotal = styled('div')(() => ({ | ||
| display: 'flex', | ||
| justifyContent: 'space-between', | ||
| fontWeight: 800, | ||
| fontSize: '1.4rem', | ||
| })); |
Copilot
AI
Aug 6, 2025
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.
SummaryTotal styled component duplicates styling that already exists in textVariants.js (SECTION_TOTAL). Consider using the existing text variant instead of creating duplicate styles.
| export const SummaryTotal = styled('div')(() => ({ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| fontWeight: 800, | |
| fontSize: '1.4rem', | |
| })); | |
| export const SummaryTotal = styled(SharedTypography).attrs({ | |
| variant: 'SECTION_TOTAL', | |
| })({ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| }); |
No description provided.