Skip to content

Feras-w3-React #21

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Feras-ALdemashki
Copy link

No description provided.

Copy link

@tkitson tkitson left a comment

Choose a reason for hiding this comment

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

Hey @Feras-ALdemashki looking good! Functionality is all there as you said - ideally the favourite context would be in its own component - at the moment the logic is split between the App.jsx file and the Cards.jsx file which makes it a little confusing. Can you see if you can update this? Let me know if you have any questions :)

import Favorite from "./pages/Favorite";
import { useState } from "react";
// i don't know why im getting an error here while everything is working fine
import { favoriteContext } from "./components/Cards";
Copy link

Choose a reason for hiding this comment

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

You are getting an error here because React components always start with a capital letter: it should be FavoriteContext (this is called pascal case)

import { favoriteContext } from "./components/Cards";

function App() {
const [favorite, setFavorite] = useState([]);
Copy link

Choose a reason for hiding this comment

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

Hey @Feras-ALdemashki nice work! The app is working but normally the context would all be defined in a separate file. Can you create a folder called 'Contexts' and move all of your favourite context logic into a component?

@tkitson tkitson self-assigned this May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants