@BerlinWebDev
Latest solutions
Latest comments
- @ToHX
Good job! Some things which I did notice:
- You're missing an h1 in your HTML
- Class naming, do you use a specific format like BEM?
- font-size always in REM
- Fallback for the font-family is missing -Instead of for example Flexbox and padding you did work with fixed width and height which makes it not responsible (not an issue in this project though)
Marked as helpful - @catherineisonline@ToHX
Hi,
you already have many comments but let me add one too :D
- font-size: 15px; should be in REM
- box-sizing: border-box is on the <body> but I think most CSS Reset will use it on the * selector
- body min-height: 100vh;
- I did use the <main> and then an <article> element,
Marked as helpful - @haikalmol@ToHX
Hi there,
as I was handing in my solution shortly after yours I was taking a look at your code as the solution does look really good.
I have some things I noticed as I am curious about your decisions :-)
- You didn't use box-sizing: border-box;
- You did give the container a vw and vh, but from what I learned width 100vw introduces overflow and instead of height 100vh you should use min height so the content can scroll when needed
- You did use a <div> before the <main> where instead I would've guessed you use it after the main
- You could give the card a max-width instead of the width
Marked as helpful - @ToHX@ToHX
Hi, thanks for you feedback!
I'm actually a bit confused, as I did learn that I shouldn't use
widthwith this card, because this will give it a fixed width which will introduce problems for the responsibility.Also, for font you should never use px, always use rem. This won't make your side inclusive.
- @hpsetti@ToHX
Hi there 👋
Some issues I saw
- your CSS class naming could be improved with BEM
- Your img has no alt Attribute
- You have a h3 but no h1
- Font size always in rem
Marked as helpful
