sushi world site #31
sushi world site #31
Conversation
index.html
Outdated
| </label> | ||
| <label> | ||
| <span>Phone</span> | ||
| <input type="number" name="phone" placeholder="+46 123456789"/> |
There was a problem hiding this comment.
Suggest you change input type to tel.
<input type="tel" name="phone" placeholder="+46 123456789"/>
index.html
Outdated
| <form> | ||
| <h2>For larger companies please call!</h2> | ||
| <p>08-1234567</p> | ||
| <form action="http://httpbin.org/anything" method="post"> |
There was a problem hiding this comment.
This post action does´t seem to work, a new tab should open to http://httpbin.org/anything. But the page just reloads. It could be that you have a form inside a form but I´m not sure.
| } | ||
|
|
||
| .flex-navi{ | ||
| display: flex; |
There was a problem hiding this comment.
Flex navi buttons don´t really fit under 1200px, consider changing the flex direction, add flex wrap or make them smaller to fit the screen.
script.js
Outdated
| @@ -0,0 +1,10 @@ | |||
| document.getElementById("hamburger").addEventListener("click", function() { | |||
| let menu = document.getElementById("nav-menu") | |||
There was a problem hiding this comment.
Consider changing let to const if you are not going to change the value.
|
Can you share your Netlify link please 😊 |
|
There was a problem hiding this comment.
Hi Oskar! Good job creating this business site 🥳 I especially like the styling of the form and the hero video! It looks super nice on mobile, but have a look on how it behaves on larger screens. It looks like your buttons are taking up to much space when they’re in a row, because you get a side scroll.
Also, one of the requirements was to create a responsive article/card grid.
The body should be a sibling to the head. Your body tag is too far up in your HTML element structure.
Don’t forget to indent each new element and to indent each attribute if you have more than two.
Remember that all things code related should be in English (even code reviews)
Changes requested
- Make it look good on screens from 320px up to at least 1600px (e.g. we don’t want any side scroll)
- Create a responsive article/card grid
- Fix your HTML tags that are in the wrong order
|
Hi Matilda,
Now I have pushed the changes you requested. I hope I haven't
missed anything.
Best regards,
Oskar
…On Tue, 4 Mar 2025 at 17:03, Matilda Brunemalm ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Hi Oskar! Good job creating this business site 🥳 I especially like the
styling of the form and the hero video! It looks super nice on mobile, but
have a look on how it behaves on larger screens. It looks like your buttons
are taking up to much space when they’re in a row, because you get a side
scroll.
Also, one of the requirements was to create a responsive article/card grid.
The body should be a sibling to the head. Your body tag is too far up in
your HTML element structure.
Don’t forget to indent each new element and to indent each attribute if
you have more than two.
Remember that all things code related should be in English (even code
reviews)
*Changes requested*
- Make it look good on screens from 320px up to at least 1600px (e.g.
we don’t want any side scroll)
- Create a responsive article/card grid
- Fix your HTML tags that are in the wrong order
—
Reply to this email directly, view it on GitHub
<#31 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJQPVUWKLN3BOZ5V47ZBEPD2SXFFFAVCNFSM6AAAAABXRRROHOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNJYGE2DENRZGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
…ved heart from emoji. Increased text size on mobile.
…on elements in navigation menu.
sushi for everyone