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

Develop #446

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

Develop #446

wants to merge 8 commits into from

Conversation

Nkinah97
Copy link

@Nkinah97 Nkinah97 commented Sep 2, 2024

@TarasHoliuk
Copy link

  1. Change page title and add favicon

  2. There is a horizontal scroll on the page because of the appearing animation (looks nice, BTW):
    image

You should set overflow-x: hidden for the page

  1. Font doesn't match the design (almost everywhere on the page):
    image

Set font-family on the body tag level and it should be inherited. Set it additionally in places it's overwritten (by default browser styles)

  1. These elements should be navigation (anchor) links:
    image

  2. it should be textarea, not input:
    image

  3. Check the previous task checklist and make sure all general rules work on this page as well. In particular form submission behavior.

  4. Check comments:
    image

  5. Layout is broken in the mobile version (because of appearing animation):
    image

Copy link

@TarasHoliuk TarasHoliuk left a comment

Choose a reason for hiding this comment

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

Check comments

@Nkinah97
Copy link
Author

Nkinah97 commented Sep 3, 2024

thank you for your comments.
I have turned off animations for mobile devices
Layout is broken in the mobile version (because of appearing animation):
AOS.init({
duration:1500,
disable: 'phone'
});

corrected your comments

@Nkinah97 Nkinah97 requested a review from TarasHoliuk September 3, 2024 12:49
Copy link

@anastasiiavorobiova anastasiiavorobiova left a comment

Choose a reason for hiding this comment

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

Great work!
Screenshot 2024-09-03 at 15 57 54
Screenshot 2024-09-03 at 15 58 02
Check the layout for tablets
Screenshot 2024-09-03 at 15 57 06
Screenshot 2024-09-03 at 15 57 27
Screenshot 2024-09-03 at 15 56 12
The image looks skewed
Screenshot 2024-09-03 at 15 55 23
The form should be cleared on the submit
Screenshot 2024-09-03 at 15 54 51
Consider removing default outlines and using input with the proper type for the email. Also, a user shouldn't be able to resize form fields. Check alignment here.
Screenshot 2024-09-03 at 15 54 43
Consider using links here and adding hover styles

@Nkinah97
Copy link
Author

Nkinah97 commented Sep 3, 2024

thanks for the feedback, I tried to fix it

Copy link

@anastasiiavorobiova anastasiiavorobiova left a comment

Choose a reason for hiding this comment

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

Great work! Almost done
Screenshot 2024-09-03 at 22 51 13
Check styles for mobiles

@Nkinah97
Copy link
Author

Nkinah97 commented Sep 3, 2024

thank you for your patience)

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.

Almost done!
To improve:

  1. You need to fix the horizontal scroll on the mobile version
image image image

@Nkinah97
Copy link
Author

Nkinah97 commented Sep 4, 2024

thanks, fixed it

Copy link

@anastasiiavorobiova anastasiiavorobiova left a comment

Choose a reason for hiding this comment

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

Awesome work!

@Nkinah97
Copy link
Author

Nkinah97 commented Sep 4, 2024

Awesome work!

thanks)

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.

4 participants