-
Notifications
You must be signed in to change notification settings - Fork 1.2k
1st variant #875
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?
1st variant #875
Conversation
Denys-Kravchuk9988
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.
Good job!
A few things to improve:
- Please use
.svgformat for icons to have better quality here. And I would recommend to displayheaderon whole page size
- Please remove default styles for button (waste border)
- Text of icons should be white and border dark
- Prices should be in a row
- Please, pay attention for margins
- Cards should be in a row
- I would recommend to decrease margin according the design
- I would recommend to change
>symbol for icon from design
- Here silver borders are cuf off. I would recommend to use silver border as dark border is used and when it's active to update the silver color to dark
- Margin between text and button is bigger than on design
- One image is absent
- Icon
$is absent and I choose 7 items of phone and the total count is still 1. Also the border around checkout block is absent
- It seems that the counter doesn't calculate price correctly in case:
- I was on cart page and pressed 7 times on plus
- Moved back on main page -> choose a new iphone and added it to cart
- On this page I see 1 Iphone 7 but price is higher than expected
|
https://beliyandr.github.io/react_phone-catalog/ I fixed your recommendetions |
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.
good job
- products in this section should not be discounted
- it would be good if the product could be removed from the cart after pressing the button again
- price placment looks broken
- you should also include the page number in the search parameters
- the image is too big at the moment, so scrolling appears
- These numbers should be identical.
- add a favicon and a title
|
https://beliyandr.github.io/react_phone-catalog/ I fixed your new recommendetions |
Anton-Kuchmasov
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.
|
https://beliyandr.github.io/react_phone-catalog/ I fixed UI issue |
Anton-Kuchmasov
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.
Well done!
Don't forget to add link to your GitHub profile in Footer (do it before final review)

https://beliyandr.github.io/react_phone-catalog/
I know that I need to add skeleton and another things (