-
Notifications
You must be signed in to change notification settings - Fork 719
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
react catalog #181
base: master
Are you sure you want to change the base?
react catalog #181
Conversation
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.
Very good job! Looks amazing 🔥
I added a few comments - check them
src/components/Category/Category.tsx
Outdated
</h2> | ||
|
||
<p className="image__subtitle"> | ||
71 models |
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.
it's better to use dynamic value
Screen | ||
{' '} | ||
<br /> | ||
{' '} | ||
Capacity | ||
{' '} | ||
<br /> | ||
{' '} |
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.
use string template
src/App.tsx
Outdated
const [likedProducts, setLikedProducts] = useState<Phone[]>([]); | ||
const [cartProducts, setCartProducts] = useState<CartItem[]>([]); |
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.
I'd recommend you create useLocalStorage
custom hook to handle values stored in the local storage instead of saving them here and passing them as props to child components
you may google it or ask in the chat if you need help
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.
Well done 👍 Added one comment - fix it (easy to do, I'm sure you will do it right - so approved)
src/components/Category/Category.tsx
Outdated
{phones.length} | ||
{' '} | ||
models |
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.
{phones.length} | |
{' '} | |
models | |
{`${phones.length} models`} |
fix all similar cases
the variant with string template looks much laconic
https://katerynashylina.github.io/react_phone-catalog