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

Assignment w1 react HANNA MELNYK #6

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

hanna-melnyk
Copy link

No description provided.

@sycons sycons self-assigned this Sep 8, 2024
Copy link

@sycons sycons left a comment

Choose a reason for hiding this comment

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

Good implementation 👍

  • Functionality to display products and filter by categories is implemented
  • Use of state hook
  • Copied fake data into src folder as instructed.

Requested improvements

  • Clean up the displayed product names and category names
  • Improve folder structure by moving components into a folder called components. This structure improves maintainability. It's good to be able to recognise the difference between pages (stand alone) and components (re-usable)

import { ProductList } from './ProductList';

export const CategoryList = () => {
const [selectedCategory, setSelectedCategory] = useState(null); // Initialize state with null to show all products
Copy link

Choose a reason for hiding this comment

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

Good use of state hook 👍

const handleCategoryClick = (category) => {
if (selectedCategory === category) {
// If the clicked category is already selected, deselect it
setSelectedCategory(null);
Copy link

Choose a reason for hiding this comment

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

Nice feature to toggle the selection on and off 👍

return (
<div>
{/* Category selection buttons */}
{categories.map((category, index) => (
Copy link

@sycons sycons Sep 9, 2024

Choose a reason for hiding this comment

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

This could be a standalone component. Give it a try and see.

<div className="product-grid">
{filteredProducts.map((product) => (
<div key={product.id} className="product">
<img src={product.image} alt={product.title} />
Copy link

@sycons sycons Sep 9, 2024

Choose a reason for hiding this comment

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

Good practice to use the alt attribute in web development 👍

{filteredProducts.map((product) => (
<div key={product.id} className="product">
<img src={product.image} alt={product.title} />
<h2>{product.title}</h2>
Copy link

Choose a reason for hiding this comment

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

Remove FAKE: from the displayed title.

onClick={() => handleCategoryClick(category)} // Handle category click
className={selectedCategory === category ? 'active' : ''}
>
{category}
Copy link

Choose a reason for hiding this comment

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

Remove FAKE: from the category name.

@hanna-melnyk
Copy link
Author

Comments cleared!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants