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

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

Develop #913

wants to merge 37 commits into from

Conversation

hardaira
Copy link

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! Check fonts here
Screenshot 2024-03-22 at 14 00 27
Screenshot 2024-03-22 at 14 00 40
The height of the form fields shouldn't jump
Screenshot 2024-03-22 at 13 47 37
Screenshot 2024-03-22 at 14 06 48
Screenshot 2024-03-22 at 14 07 09
Check the layout for the tablet. When the menu is open the page shouldn't be scrollable. Also, consider making hover effects smooth

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.

Try to fix demo link

image

@hardaira hardaira requested review from GUSILLUS March 23, 2024 13:24
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.

Still not working((

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.

Needs some improvements.

  • On some width (1920px on screenshot) layout is broken and doesn't match design
    Screenshot from 2024-03-23 17-14-47
    Screenshot from 2024-03-23 16-57-02
    Screenshot from 2024-03-23 16-43-50
  • add transition for elements with hover effect to make change styles smoothly.
    for example
    chrome-capture-2024-3-23

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.

Looks like still broken layout on demo link.
Did you make deploy after fixes?
Screenshot from 2024-03-23 18-58-50

Please try to deploy again. If will have some problems - feel free to ask in fe_chat and check your demo link before next re_request.

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 should fix prev comments. Please feel free to ask for help in chat.

@hardaira hardaira requested a review from vadiimvooo April 4, 2024 10:47
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:

  1. Try to make slider workable
    image

  2. Add some alert logic on this button
    image

  3. Add transition duration on every hover effect to improve UI

  4. Try to improve hover effects on card, make it look like a group effect. Because now you have one for arrow one for text one for whole card
    image

  5. Fix form UI
    before
    image
    after click
    image

  6. Need to be a social links not a gh
    image

  7. Fix error
    image
    image

  8. Make link text and href text the same
    image

@hardaira hardaira requested a review from GUSILLUS April 15, 2024 16:07
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.

Good job, but still some comments is not fixed.

  1. Add transition on hover effect for all interactive elements (for example links in header, buttons etc)
  2. It should not move any elements when you focus input in form.
    image
  3. This tab still redirect to error page. Please fix it
    image
  4. Provide close button in menu
    image
  5. Space between sections is bigger then it should be, please check the mockup (mobile version)
    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.

lgtm

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