This is my second solution. All kind of feedback is welcome. Thank you
lepamoore
@lepamooreAll comments
- @Cool17Submitted over 1 year ago@lepamoorePosted over 1 year ago
Hey Cool17,
nice Solution! I'm just looking at your code and I noticed some things which I'd like to share with you.
You're using semantic HTML, which is great.
For the body CSS Rule, you should use a height of 100% instead of 100vh. When using 100vh on mobile devices, the browser bar overlaps the content. When using 100% instead, this won't happens since the content will stop before the browser bar.
For the nav CSS Rule, you set the display attribute to flex. But you didn't set any other rules in connection with flex, like align-items or justify center. Instead, you gave your .home container a huge left margin. Instead of the hard coded margin, you could've used something like:
nav: { display: flex; align-items: center; justify-content: space-between; }
Align-items centers all items in nav vertically, and justify content creates as much space as possible between the image and the nav list (your home container).
If you want to place some block-level (like div or li) items next to each other, give them a width and display: inline-block; property. That way you can place them next to each other and still style them with margins etc. like block elements.
I recommend using rem units for font-size. Standardized 1rem equals 16px if it isn't changed in the HTML CSS Rule, because of that you can do 18(px) divided by 16 equals 1.125(rem).
your HTML code looks valid to me. Here's just one thing, use the alt attribute on img elements for accessibility and SEO reasons.
and you could use min-width more often so the details containers won't get compressed so much. e.g. for the black New container min-width: 300px; so it doesn't shrink smaller than 300px.
The last issue is that your black New container runs across the webpage when you're resizing the window. To be honest, I don't know why this is happening, but I think It's because the grid isn't set up properly. I think the image isn't in the grid etc. but I could be wrong, I hate CSS grid lmao.
I think these 2 resources do have a huge worth.
-
First a YouTube video about CSS grid in this video there's a grid technique shown which is pretty much the easiest way to use grid, but almost nobody teaches it: Grid YT Video.
-
The second is Flexbox Froggy a really nice way to practice flexbox skills and getting familiar with all opportunities flexbox gives you Flexboxfroggy.
I hope you like my feedback, although I'm not a pro as well.
Happy Coding!
0 -
- @LandielDuransSubmitted over 1 year ago
Hello, I'm Brazilian, and this is the first project that I do and share, everything is still new to me, I hope I can count on the help of the community and that they can help me grow even more in this journey as a developer in development.
@lepamoorePosted over 1 year agoHey Landiel π, Your code looks very nice ! Much better than my first project here, lmao. I just read it and want to give you some feedback.
-
First, for body HTML and DIV tags, you don't need to specify the width to 100%. For these elements, the width is automatically set to 100%.
-
Then I recommend using the units % or vw (view-width) and vh (view-height) for width and height of a div or a similar element. When you use the % unit, e.g. width: 50%; Your element will be as half as big as the parent element because the % unit takes the height or width value of the parent as 100%. When you use width: 50vw; it's the 50% of the viewpoint width or device width. Same for height: 50vh; it's 50% of the viewpoint width or device width.
-
When you use these relative units, your elements will grow and shrink as the browser does. You can set maximum or minimum values in px. you could use this CSS rule as an example:
div {height: 50vh; min-height: 200px;}
in this case, your div container is half as big as the height of the browser window, but it won't get smaller than 200px. That means, even if your browser window is 300px high, your div will remain 200px high. -
I saw you used some flexbox to center your divs. That was pretty nice, i just noticed you only used justify-content with flexbox. justify-content is as you know for the horizontal alignment. And if you use align-items: center; as well, your element will be fully centered.
<div id='parentDiv'> <div id='childDiv'></div> </div>
#parentDiv { display: flex; align-items: center; justify-content: center; background-color: blue; } #childDiv { height: 50%; width: 50%; background-color: white; }
-
above you can see the code for a fully centered div in another one. Feel free to copy it and run it in the browser to get a better understanding of what I mean. My English isn't the best, so I hope you can understand what I mean π .
-
another important thing is use for further description of an image, an alt attribute. don't use title='this image shows a chair'. Use alt='this image shows a chair'. This way people who are using the internet with screen readers can hear your pictures then instead of seeing them.
-
keep it up. I'm just new to coding like you, but I hope my hints or tips were helpful to you. Greetings and Happy coding! π
0 -