Concert House #364
Concert House #364
Conversation
There was a problem hiding this comment.
Good job with this business site Gitte! As I mentioned on your demo, I really like the yellow on the black/white picture 🥳 Nice and accessible contrasts.
HTML
- The header includes a logo and text elements wrapped in a div with the class hero, which is a nice, organized approach. However, instead of using a custom
<logo>element, it's better to stick to standard HTML tags like<div>or<header>, as<logo>is not a recognized HTML tag. - For the links inside the navigation, make sure you include text between the tags for better accessibility and user interaction. Right now, they seem to be empty. And no need for the
<ul>element.
CSS
- I'm not a designer but I've learnt a thing or two, so one tip I would like to give you is to align all your elements (labels have a left padding and inputs don't)
- Adding some padding around the placeholders in the input would look nice
- I think the yellow color you chose for the hero would be a nice contrasting color to play around with in the form as well. Maybe as a background color on the button and potentially as a border or outline/focus too?
Overall, you're on the right track, and your project is nicely executed! Keep up the great work! 🎉
There was a problem hiding this comment.
-
Clear Structure: Nice use of semantic tags like header, footer, and proper form elements like label and input. It makes the code easier to follow and better for search engines.
-
Mobile Friendly: The media queries you've added do a great job making sure the form and header look good on different screen sizes.
-
User-Friendly: Adding the required attributes to the form fields is a great touch—it ensures users fill in all the important info without missing anything.
|
|
||
| <!-- video or image as a header is cool :) --> | ||
| <head> |
There was a problem hiding this comment.
There are duplicated head and body tags, which can cause rendering issues.
| /* footer for mobile phone */ | ||
| footer { | ||
| display: flex; | ||
| display: row; |
There was a problem hiding this comment.
display: row; isn't a valid value. Correct way to set row layout (default for flex): flex-direction: row;
code/index.html
Outdated
| <div class="hamburger-layer"></div> | ||
| <nav> | ||
| <ul> | ||
| <a class="a-header" href="http://google.com"></a> |
There was a problem hiding this comment.
The elements within the navigation don’t have visible text. This might confuse users and affect accessibility.
https://concert-house.netlify.app/