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 #2589

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

Develop #2589

wants to merge 87 commits into from

Conversation

wsadocha
Copy link

@wsadocha wsadocha commented Oct 2, 2023

@wsadocha wsadocha requested a review from loralevitska October 17, 2023 11:05
Copy link

@loralevitska loralevitska left a comment

Choose a reason for hiding this comment

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

You did an amazing job! Looks good! Let's do some improvements

  • add transition for both sides (use this property for element, not just for hover)
  • make all form field required

@wsadocha wsadocha requested a review from loralevitska October 19, 2023 08: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.

  1. Need to remove the autocomplete styles
image
  1. The user will not be able to change the size of this field
image
  1. When I click send form I get this problem
image

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.

It was a pleasurable experience to interact with your page! I like smooth hovers and high-quality images here. The layout for mobiles and desktops is awesome
Screenshot 2023-10-19 at 14 56 46
Consider fixing validation errors for proper rendering in the browser
Screenshot 2023-10-19 at 14 53 07
The menu in landscape mode should be scrollable (use checklist - 18)
Screenshot 2023-10-19 at 14 53 33
Screenshot 2023-10-19 at 14 53 57
The container for tablets should be wider to meet the Figma design

@wsadocha
Copy link
Author

I figured out how to reset form fields without refreshing whole site. That's why I did another commit after I requested a review :)

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.

  1. Still not fixed from the previous review
image
  1. The image for the header needs to be imported in better quality
image

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! You have done a lot of work here! 💪

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.

6 participants