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

Cart #15

Merged
merged 13 commits into from
Jun 26, 2024
Merged

Cart #15

merged 13 commits into from
Jun 26, 2024

Conversation

AYOPELUMI
Copy link
Collaborator

  • Created the Cart Context API
  • Added the Context wrapper in the 'App.jsx' file
  • Moved the 'products.json' file to the public folder
  • Fixed the bugs in the Product component
  • Added the 'add to cart' functionality to the productCard component
  • Added Cart to the routes for

@AYOPELUMI AYOPELUMI added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 23, 2024
@Divinechi
Copy link
Collaborator

Divinechi commented Jun 24, 2024 via email

Comment on lines 1 to 64
/* Cart.css */

.cart-container {
padding: 20px;
background-color: #f9f9f9;
border: 1px solid #ddd;
border-radius: 5px;
margin-bottom: 20px;
}

.cart-container h2 {
font-size: 1.5em;
margin-bottom: 20px;
}

.cart-container p {
margin: 5px 0;
}
Cart.css */

.cart-container {
padding: 20px;
background-color: #f9f9f9;
border: 1px solid #ddd;
border-radius: 5px;
margin-bottom: 20px;
}

.cart-container h2 {
font-size: 1.5em;
margin-bottom: 20px;
}

.cart-container p {
margin: 5px 0;
}

.cart-container ul {
list-style-type: none;
padding: 0;
}

.cart-container li {
border-bottom: 1px solid #ccc;
margin-bottom: 10px;
padding-bottom: 10px;
}

.cart-container button {
margin-right: 10px;
background-color: #007bff;
color: white;
border: none;
padding: 5px 10px;
cursor: pointer;
}

.cart-container button:hover {
background-color: #0056b3;
}




Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AYOPELUMI
@Divinechi
@adex-badr18

Weldone, implementing the project requirements. You all agreed to use tailwind CSS for the implementation of the styles but here you went ahead to use vanilla CSS. Please kindly redo this part of the project.

Copy link
Owner

@Megagig Megagig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AYOPELUMI @Divinechi
Great job so far! 👏 You've done well, and you're almost there for the next project ⏭️, but there are a few issues 🐛 that you still need to work on.

image

Don't Worry 🙏
It's okay to have bugs in your code. 🆗 Please see this code review as an opportunity to learn practical coding skills and discover new tips that can improve your code quality 💯 and help you get even better 💼.

I will guide you on how to address these areas down below 👇.

Highlights
✅ everyone has contributed

Required Changes ♻️
Check the comments under the review.

Optional suggestions
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

Cheers and Happy coding!👏👏👏

Have any doubt ❓
Feel free to leave any questions or comments in the PR thread if something is not 100% clear. Please, remember to tag me in your question so I can receive the notification.

⚠️ WARNING ⚠️
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

@@ -0,0 +1,87 @@
/* eslint-disable react/prop-types */
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AYOPELUMI
Here you disabled the eslint from the project. please kindly do your implementation without disabling the eslint. the eslint helps us write good codes that are modular and obeys standards.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the insights
I will do that after fully pleased that all basic functionality and design layout are met

Comment on lines 3 to 4
import './Cart.css'; // Import the CSS file for styling

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Divinechi Please kindly style the project with Tailwind as we all agreed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I have done that thank you, you can review it now.

@Megagig
Copy link
Owner

Megagig commented Jun 25, 2024

@Divinechi I am sorry we cannot merge your work into the development branch. This is because your styling has affected the initial work carried out on the project. Have a look at the screenshot of your styles.

Screenshot from 2024-06-25 07-53-26

I have disabled the ability for anyone to merge pull requests. imagine what could have happened if this pull request is merged without review. it will just revert us to square one.

Screenshot from 2024-06-25 07-55-13

Screenshot from 2024-06-25 07-55-48

Screenshot from 2024-06-25 07-56-47

Please don't merge this pull request.
Thanks

@Megagig Megagig self-requested a review June 25, 2024 06:58
Comment on lines 1 to 2
export default {
module.exports = {
plugins: {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you tamper with the Tailwind config? @Divinechi

Copy link
Owner

@Megagig Megagig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Divinechi I am sorry we cannot merge your work into the development branch. This is because your styling has affected the initial work carried out on the project. Have a look at the screenshot of your styles.

Screenshot from 2024-06-25 07-53-26

I have disabled the ability for anyone to merge pull requests. imagine what could have happened if this pull request is merged without review. it will just revert us to square one.

Screenshot from 2024-06-25 07-55-13

Screenshot from 2024-06-25 07-55-48

Screenshot from 2024-06-25 07-56-47

Please don't merge this pull request.
Thanks

@Megagig
Copy link
Owner

Megagig commented Jun 25, 2024

@iyke-e I saw the commit on tailwind config file which you pushed. This is to bring to your notice that the styling errors are not yet sorted out. Please kindly implement this. If you are having issues that you cannot solve, reach out to team members to help you get unstuck

@iyke-e
Copy link
Collaborator

iyke-e commented Jun 25, 2024

Noted we're working on it

@Megagig
Copy link
Owner

Megagig commented Jun 26, 2024

@Divinechi @AYOPELUMI @iyke-e

Weldone, on your awesome work.

Project is approved as agreed by the reviews during the call today.

congratulations

@Megagig Megagig merged commit dfa0f83 into development Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants