-
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
Responsive header block complete #2671
base: master
Are you sure you want to change the base?
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.
Hey, there isn’t much to review at the moment. You can request a review after you’ve finished or once you’ve implemented something significant, such as a complete landing page for desktop or mobile, etc.
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 should be phone number on hover:
- Change text color on hover for phone, email and address:
- Make the phone number and email clickable to start a phone call or compose an email
- Form shouldn't be submitted if some of the fields are not filled
- Page shouldn't be reloaded on form submit (https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault)
- Prevent menu links from pushing down other elements when hovered due to the appearance of border-bottom
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.
after form submission page redirects to 'Page Not Found' error. you should add js script to prevent page from reloading (https://www.w3schools.com/Jsref/event_preventdefault.asp).
additionally consider clearing inputs (https://www.w3schools.com/Jsref/met_form_reset.asp) or displaying a notification to indicate that submission was successful
@@ -312,5 +314,7 @@ <h2 class="contact__title">Contact us</h2> | |||
</main> | |||
|
|||
<footer class="footer"></footer> | |||
|
|||
<script src="./scripts/main.js"></script> |
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.
masz już podpięty ten skrypt na samej górze w head przez co alert wywołuje się podwójnie ^^"
src/styles/blocks/header.scss
Outdated
font-size: 64px; | ||
line-height: 64px; | ||
|
||
padding-top: 294px; |
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.
src/styles/blocks/header.scss
Outdated
|
||
&__icons { | ||
width: 96px; | ||
height: 96px; |
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.
src/styles/blocks/header.scss
Outdated
|
||
&__icons { | ||
width: 96px; | ||
height: 96px; |
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.
height: 96px; |
src/styles/blocks/header.scss
Outdated
font-size: 80px; | ||
line-height: 80px; | ||
|
||
padding-top: 294px; |
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.
padding-top: 294px; | |
padding-top: 336px; |
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.
Great work! 🚀
DEMO LINK