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

W3 aziz react #29

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

Conversation

azizsigar
Copy link

No description provided.

@robvk robvk self-assigned this Nov 27, 2024
Copy link

@robvk robvk left a comment

Choose a reason for hiding this comment

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

Some changes, and I don't think I saw a link to a deployed version, is that correct?

const [selectedCategory, setSelectedCategory] = useState(null); // Başlangıçta kategori seçilmemiş

// Kategori seçildiğinde ürünleri filtrele
const handleCategoryClick = (category) => {
Copy link

Choose a reason for hiding this comment

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

Why do you need this function? It only calls one other function

<div className="category-list">
{categories.map((category, index) => (
<button key={index} onClick={() => {
console.log("Category clicked: ", category); // Tıklanan kategoriyi kontrol edelim
Copy link

Choose a reason for hiding this comment

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

Remove console logs from the code when creating a PR

<FavoritesProvider>
<Router>
<NavBar />
<Routes>
Copy link

Choose a reason for hiding this comment

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

Clean, nice!


const ProductCard = memo(({ product }) => {
const { favorites, toggleFavorite } = useFavorites();
const isFavorite = favorites.includes(product.id);
Copy link

Choose a reason for hiding this comment

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

You are doing this in two places, maybe nicer to have the Favorites context provide this function?

dispatch({ type: "TOGGLE_FAVORITE", payload: id });
};

const value = useMemo(
Copy link

Choose a reason for hiding this comment

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

Why does this need to be memoized?


const initialState = [];

const favoritesReducer = (state, action) => {
Copy link

Choose a reason for hiding this comment

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

Little over the top to use a reducer here but cool that you are trying it!


useEffect(() => {
const fetchFavorites = async () => {
const fetchedProducts = await Promise.all(
Copy link

Choose a reason for hiding this comment

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

Nice use of Promise.all!

@azizsigar azizsigar requested a review from robvk December 1, 2024 14:03
Copy link

@robvk robvk left a comment

Choose a reason for hiding this comment

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

Good job

@robvk robvk added Approved and removed Needs work labels Dec 2, 2024
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