Business site / contracting #48
Conversation
…fects on the images, text-decoration on the menu.
|
link to the site: https://bb-contracting.netlify.app/ |
Nicolinabl
left a comment
Nicolinabl
left a comment
There was a problem hiding this comment.
(I just realized i wrote all the comments in english so i will continue that way in case anyone wants to read this)
Really nice work! The site has a good layout and the responsiveness works great, site looks very good in all sizes. I specifically really like that you put the form on a different site/page accessible via the contact link, that makes it feel very professional. Also nice work that you made the logo to a link that returns to homepage, nice touch. The form is nice and simple design, and fully functional when tested. Apart from it saying the form is not safe, i have that same issue on mine and dont understand why.
When clicking "service", the full page goes blank. But maybe thats a work in progress and not a bug.
I have added some comments in the code to review, nothing major. All in all really good work :)
| <!--aria-hidden="true"--> | ||
| <body> | ||
| <!--header Menu list nav bar--> | ||
| <header class="header"> |
There was a problem hiding this comment.
Maybe you don´t need a class on the header. As the class name "header" is the same as the element name you should be able to style it without adding a class
| @@ -1 +1,26 @@ | |||
| # js-project-business-site | |||
| # This is my Business site | |||
|
|
||
| <form action="http://httpbin.org/anything" method="POST"> | ||
| <h1>Contact Us</h1> | ||
| <div class="contact-form"> |
There was a problem hiding this comment.
Here you could use the <form> tag instead of making it a div
| </ul> | ||
| </div> | ||
|
|
||
| <nav class="desktop-nav" aria-label="Main"> |
There was a problem hiding this comment.
nice choice of class name. Makes it easy to follow and understand that this is the navigation bar that shows in desktop and not mobile
| <div class="contact-form"> | ||
| <label for="name">Name:</label> | ||
|
|
||
| <input type="text" id="name" name="name" required> |
There was a problem hiding this comment.
You might need more input types to meet the criteria, perhaps some radio buttons or select option? :)
index.html
Outdated
| </div> | ||
| </a> | ||
|
|
||
| <a href="services.html" class="grid-card" style="background-image: url(./assets/img/under-construction-2891888_1280.jpg);"> |
There was a problem hiding this comment.
Nice formatting on the code. It is easy to follow along :)
| transform: translate(-50%, -50%) rotate(-45deg); | ||
| } | ||
|
|
||
| /*desktop*/ |
There was a problem hiding this comment.
Great use of comments to show what parts you're styling
| text-align: center; | ||
| } | ||
|
|
||
| /* Grid – mobil: 1 kolumn */ |
There was a problem hiding this comment.
again, very clear with the comments :)
| color: black; | ||
| } | ||
|
|
||
| /* === Tablet (≥600px) === */ |
There was a problem hiding this comment.
Interesting that you collected all the media queries down here. I wasnt sure myself where to put them in mine. I first put them all at the bottom like you but then spread them out to where they "belong". Im not sure which is the "correct" approach. Its maybe easiest to find them if they are a grouped together like you did. Will look into further
| z-index: 2; | ||
| } | ||
|
|
||
| .ham-menu span:nth-child(1) { |
There was a problem hiding this comment.
Im impressed that you used :nth-child(), i found that a bit complicated myself
HIPPIEKICK
left a comment
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Nice and clean JS code that works as expected ⭐ You’ve implemented CSS Grid and Flexbox in a good way as well.
Keep up the good work!
| </div> | ||
| </section> | ||
|
|
||
| <form action="http://httpbin.org/anything" method="POST"> |
There was a problem hiding this comment.
Make sure you're using the https method, otherwise most browsers will block this action
| <li><a href="index.html">Home</a></li> | ||
| <li><a href="services.html">Service</a></li> | ||
| <li><a href="contact.html">Contact</a></li> | ||
| </ul> | ||
| </div> | ||
|
|
||
| <nav class="desktop-nav" aria-label="Main"> | ||
| <ul> | ||
| <li><a href="index.html">Home</a></li> | ||
| <li><a href="services.html">Service</a></li> | ||
| <li><a href="contact.html">Contact</a></li> |
3 participants
This is my Business site atm, still not done yet.