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

first-phone-catalog #490

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Klimukalina
Copy link

@Klimukalina Klimukalina commented Aug 25, 2024

Посилання на github: https://github.com/Klimukalina/react-project
Посилання на сторінку: https://klimukalina.github.io/react-project/

Copy link

@volodymyr-soltys97 volodymyr-soltys97 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!
Let's improve your code

  1. Fix this text to Phones
image
  1. Open this additional link in the new tab
image
  1. Fix the tablet version
image
  1. Also need to fix the mobile version
image

Copy link

@GUSILLUS GUSILLUS left a comment

Choose a reason for hiding this comment

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

GJ!

1.Consider increase step og slider form 1 to 4 per time
image

  1. Make 4 by default
    image

  2. Try to center name of page
    image

  3. Why you need this button if you don't have any scroll?
    image

Copy link

@GUSILLUS GUSILLUS left a comment

Choose a reason for hiding this comment

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

GJ!

  1. Fix logic to show only 4 item by default
    image

  2. Make this workable
    image

  3. Don't have scroll but have button
    image

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

GJ!
To improve:

  1. Add a favicon on the page
image
  1. Open this additional link in the new tab
image

Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

  1. better to display some difference between the disabled button and the active one
image 2. don't you want to implement those selectors? image

Copy link

@GUSILLUS GUSILLUS left a comment

Choose a reason for hiding this comment

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

GJ!

But try to fix your demo link
image

Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

when I add some product to the cart and reload the page, the button looks like this product is not in the cart
https://github.com/user-attachments/assets/6c9a9b3a-f183-42b3-a454-17ff79a9dfcd

  1. better to implement it
image

Copy link

@VitaliyBondarenko1982 VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

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

Almost done.
Needs some fixes on product details page.

  • available color and capacity should be clickable and lead to product page with such parameters.
  • favorite button not works on product details page
Screen.Recording.2024-09-29.at.17.11.43.mov
  • looks like broken logic on click to small product images. they shouldn't change, after click should show selected image on big image, selected image should have have selected styles (see design)
Screen.Recording.2024-09-29.at.17.13.14.mov
  • add to cart button also should remove product from cart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants