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

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

Develop #2591

wants to merge 29 commits into from

Conversation

Krzychu92
Copy link

@Krzychu92 Krzychu92 commented Oct 11, 2023

https://krzychu92.github.io/layout_miami/

Mobile block (320px)
Tablet block (744px)
Mobile (1260px)

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.

This link doesn't working(
You need to copy the entire link and paste it here
image

@Krzychu92
Copy link
Author

This link doesn't working(
You need to copy the entire link and paste it here
image

Updated

@Krzychu92
Copy link
Author

image
I can't sort out why the layout is not scaling down :/

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.

Good job! There are things that should be fixed:

  • add favicon and change title
    image
  • implement this logic (by hover on phone icon) and make this phone-number reachable
    image
  • implement all modes (tablet, desktop)
    image
  • add hover effects for images in gallery
  • remove outline and default background color after autocomplete, it breaks design
    image
  • make all form fields required
  • make sure that all interactive elements have a cursor pointer on hover
  • make it links
    image

@Krzychu92
Copy link
Author

@loralevitska that is a mobile version thats why I havent done any hover actions. The task says to send PR after each block is done so I sent a mobile version to check if its okay to keep going xd

@etojeDenys
Copy link

@loralevitska that is a mobile version thats why I havent done any hover actions. The task says to send PR after each block is done so I sent a mobile version to check if its okay to keep going xd

next time, write briefly about what you implemented

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.

good job, keep going

  1. disable scroll when menu is open
image
  1. the button should look like this on hover
image

@Krzychu92
Copy link
Author

Added a tablet and desktop version. Please check

@Krzychu92 Krzychu92 requested a review from etojeDenys October 26, 2023 18:43
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. Add a favicon and actual title on the page
image
  1. Need to remove the scroll when this menu is open
image
  1. The prices should be here on the same line
    Demo
image

Design
image

  1. This image should be the full width of the browser window
    Demo
image

Design
image

  1. The user will not be able to change the size of this field
image
  1. Add cursor pointer and transition for hover effects here
image
  1. Make all these fields are required before sending
image
  1. These elements should be clickable
image
  1. Need to remove autocomplete styles and fix the indents for the textarea
image

@Krzychu92
Copy link
Author

I think I have changed everything from list. I couldn't find how to block scrolling without JS so I setup menu over a layout with z-index

Copy link

@nazarmatsevych nazarmatsevych left a comment

Choose a reason for hiding this comment

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

Looks great, let's make some improvements:

  1. Page should not reload on submit, also clear form fields on button click (
    add this to your form
    image
    )
  2. Open external link like googlemap in a new tab
    image

@Krzychu92
Copy link
Author

done it. Now I think is good :D

Copy link

@nazarmatsevych nazarmatsevych left a comment

Choose a reason for hiding this comment

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

👍

Copy link

@nazarmatsevych nazarmatsevych left a comment

Choose a reason for hiding this comment

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

Awesome 👍

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