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.

Sign up for GitHub

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

Jump to bottom

Develop #2586

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

Develop #2586

wants to merge 9 commits into from

Conversation

LabPetro
Copy link

Anastasiia-Svintsova
Anastasiia-Svintsova suggested changes Sep 23, 2023
View reviewed changes
Copy link

@Anastasiia-Svintsova Anastasiia-Svintsova left a comment

Choose a reason for hiding this comment

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

Great start! Pay attention to the details, please. Imagine that you're the user and test it more thoroughly.

  • The page shouldn't reload on the form submit. Just clear the form.
  • The layout shouldn't move while the submit button hovering
2023-06-08.130254.mp4

  • Add transition on hover for all elements that change style on hover

  • Use the correct position for the numbers
    image

  • Fix the logo link

2023-06-08.130254.mp4
  • Use the SVG format for the logo and icons to get better quality
  • Fix the close icon as well
2023-06-08.130254.mp4

anastasiia-tilikina
anastasiia-tilikina suggested changes Sep 26, 2023
View reviewed changes
Copy link

@anastasiia-tilikina anastasiia-tilikina 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!

  1. set the same transition duration for all elements on the page.
  2. you don't need to set a transition for hover selector only for default selector

image

  1. fix font styles for inputs. They are not inherited by default

lerastarynets
lerastarynets suggested changes Sep 26, 2023
View reviewed changes
Copy link

@lerastarynets lerastarynets left a comment

Choose a reason for hiding this comment

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

Pls watch the video review via the link :)

lerastarynets
lerastarynets suggested changes Sep 27, 2023
View reviewed changes
Copy link

@lerastarynets lerastarynets left a comment

Choose a reason for hiding this comment

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

Almost there, just one problem left)
pls fix autofill styles
Screenshot at Sep 27 15-10-20

anastasiia-tilikina
anastasiia-tilikina suggested changes Sep 27, 2023
View reviewed changes
Copy link

@anastasiia-tilikina anastasiia-tilikina left a comment

Choose a reason for hiding this comment

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

your inputs still have the wrong font styles. They're not inherited by default.
image

anastasiia-tilikina
anastasiia-tilikina approved these changes Sep 27, 2023
View reviewed changes
Copy link

@anastasiia-tilikina anastasiia-tilikina left a comment

Choose a reason for hiding this comment

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

Congrats! 🥳
You did it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anastasiia-tilikina anastasiia-tilikina anastasiia-tilikina approved these changes

@Anastasiia-Svintsova Anastasiia-Svintsova Awaiting requested review from Anastasiia-Svintsova

@lerastarynets lerastarynets Awaiting requested review from lerastarynets

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LabPetro @lerastarynets @Anastasiia-Svintsova @anastasiia-tilikina