-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
base: master
Are you sure you want to change the base?
Develop #2591
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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
- implement this logic (by hover on phone icon) and make this phone-number reachable
- implement all modes (tablet, desktop)
- add hover effects for images in gallery
- remove outline and default background color after autocomplete, it breaks design
- make all form fields required
- make sure that all interactive elements have a cursor pointer on hover
- make it links
@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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a tablet and desktop version. Please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add a favicon and actual title on the page
- Need to remove the scroll when this menu is open
- The prices should be here on the same line
Demo
- This image should be the full width of the browser window
Demo
- The user will not be able to change the size of this field
- Add cursor pointer and transition for hover effects here
- Make all these fields are required before sending
- These elements should be clickable
- Need to remove autocomplete styles and fix the indents for the textarea
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done it. Now I think is good :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome 👍
https://krzychu92.github.io/layout_miami/
Mobile block (320px)
Tablet block (744px)
Mobile (1260px)