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

Eco cosmetic was created #380

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

Conversation

Serhii-Khobotov
Copy link

@Serhii-Khobotov Serhii-Khobotov commented Apr 4, 2024

https://
Serhii-Khobotov.github.io/Eco_cosmetics/

Copy link

@SanyaBratashchuk SanyaBratashchuk left a comment

Choose a reason for hiding this comment

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

Please, provide demo link

@Serhii-Khobotov
Copy link
Author

https://
Serhii-Khobotov.github.io/Eco_cosmetics/

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

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

Well done. Just a few fixes:

  1. Please add transition on hover effect for this element
    image
  2. Add this link to the description of your PR, also please use link markup provided by github.
    image

@Serhii-Khobotov
Copy link
Author

Sorry, i forget how to do that

@Serhii-Khobotov
Copy link
Author

Copy link

@IvanFesenko IvanFesenko 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, Check this issues:

  • favIcon is epsent
  • Logo didn't match design
    2024-04-05_15h06_54
    2024-04-05_15h06_03
  • if you add validation user shoud know what format should be
    2024-04-05_15h07_45
  • On hover line should be same width as text
    2024-04-05_15h09_13

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 page

  1. Add a favicon on the page
image
  1. Add cursor pointer for all clickable elements
image

README.md Outdated
Comment on lines 60 to 61
[DEMO LINK](https://
Serhii-Khobotov.github.io/Eco_cosmetics/).

Choose a reason for hiding this comment

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

Fix this link and update it in the PR description

Suggested change
[DEMO LINK](https://
Serhii-Khobotov.github.io/Eco_cosmetics/).
[DEMO LINK](https://Serhii-Khobotov.github.io/Eco_cosmetics/).

@Serhii-Khobotov
Copy link
Author

@Serhii-Khobotov
Copy link
Author

image

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

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

You have to push your favicons. Also you didn't add cursor pointer as was mentioned before. Please feel free to ask for help in fe_chat if you need so.

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

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

Well done!)

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