-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Phone Catalog solution #883
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
base: master
Are you sure you want to change the base?
Phone Catalog solution #883
Conversation
|
Some tests are failing, waiting for mentor feedback |
FaiHamid
left a comment
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.
Hi,
Add DEMO_LINK to the PRs description
https://sashkatmshchk-pixel.github.io/react_phone-catalog/ DEMO LINK |
2pasha
left a comment
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.
hi!
i can see that your design is completely different from mockups, but okay, lets go through your app. Here is some improvements:
- add favicon
- add content container
- add sort by newest
-
add how many items can be on one page
-
some images are broken
- in Cart user can not see total price and can not change item's quantity
| style={{ | ||
| border: '1px solid #ccc', | ||
| padding: 12, | ||
| marginBottom: 12, | ||
| }} |
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.
remove inline styles, it's better use classes
src/pages/Product.tsx
Outdated
| style={{ | ||
| cursor: 'pointer', | ||
| border: | ||
| i === imageIndex | ||
| ? '2px solid black' | ||
| : '1px solid #ccc', | ||
| }} |
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.
remove inline styles, it's better use classes
etojeDenys
left a comment
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.
- you should save these values in the search parameters so that they are kept after the page is refreshed
- right now is so hard to see that disabled 1
- this page looks so raf. add some
- this page looks so raw (the right part is empty and some of the images are big and some are small)
- this card should link to the product page
- create a solution for increasing the number of products
|
Hello, - fixed.
…On Sun, 8 Feb 2026 at 11:30, Denys Semtso ***@***.***> wrote:
***@***.**** requested changes on this pull request.
1. you should save these values in the search parameters so that they
are kept after the page is refreshed
image.png (view on web)
<https://github.com/user-attachments/assets/cfcbcbd7-f801-48f5-a61a-fee73e74a194> image.png
(view on web)
<https://github.com/user-attachments/assets/ca33dd73-a1e7-405a-a22e-c3f3f42fae6e>
2. right now is so hard to see that disabled 1
image.png (view on web)
<https://github.com/user-attachments/assets/924fbb46-9a9a-435a-995b-27c9da29323e>
3. this page looks so raf. add some
image.png (view on web)
<https://github.com/user-attachments/assets/dcbb48ed-2946-4712-9615-5d42d9370d06>
4. this page looks so raw (the right part is empty and some of the
images are big and some are small)
image.png (view on web)
<https://github.com/user-attachments/assets/5ed84103-1d6f-41a0-a128-1d4912b0b50e>
5. this card should link to the product page
image.png (view on web)
<https://github.com/user-attachments/assets/98ecc7c9-f2f3-4ef2-87ad-004a26d5f9f0>
6. create a solution for increasing the number of products
image.png (view on web)
<https://github.com/user-attachments/assets/fbfb5850-0d03-4070-be89-3c3cdd31309f>
—
Reply to this email directly, view it on GitHub
<#883 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B4J7IH5KG7A7IGO27J2VSWT4K363XAVCNFSM6AAAAACT6PGLMCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTONRZGM4DONZQG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi! All changes is ready for review.
https://sashkatmshchk-pixel.github.io/react_phone-catalog/