Hi, I hope you find this feedback helpful. I'll just list it out so hopefully it's easier to follow:
- Don't include empty divs in html. There is no reason to use background image over image for this card, and it would have a negative impact on performance if you did this on this kind of component in a real site.
- Think about the context of where this component would be used when you build small components like this. This card would very likely be alongside other blog cards —multiple would be used on one page. it is extremely unlikely that this type of card would ever act as a page title which tells you it must not have a H1. H2 would be much more appropriate.
- There’s currently no way for anyone to access the blog being signed posted by this card because you’ve not included an anchor link anywhere. There should be a link inside the H2 wrapping the blog title text. (Always test solutions with your keyboard as a good step to ensuring you've used interactive elements correctly)
- to make the whole card clickable you would make the card position relative, then add a pseudo element on that link, absolutely positioned and sized to fill the space of the card.
- "Author image" is not an appropriate alt description of the image. alt should never include words like "image" because it’s already got an image role. take a look at the excellent post in the resources channel on discord about how and when to write good alt text.
- The author name is definitely not a H2. Using headings appropriately is very important. They must only be used when something is an actual heading for content underneath it. this author name is just a paragraph.
- get into the habit now of always, including a full modern CSS, reset at the start of the styles in every project. Andy Bell or Josh Comeau, both have good examples you can look up and use.
- there is a bug in your code caused by limiting the height of the body to 100 VH never limit the height of elements that contain text, including the body. Instead use min height. this will allow the body to extend beyond the limit of the viewport height when necessary.
- The main landmark should not be your card component. This component would sit on a page within main. Therefore, the component must be its own div with a class so it can be styled independently.
- there is no benefit to making the card into a flex column, unless you’re planning on using the gap property. all of the child elements within the card stack vertically by default.
- The card image must not have a explicit width. Once you have changed the image to be an img element instead of background image the sizing would be simple: A standard part of the CSS reset would be to set image elements to have a max width of 100%. That is all this image needs. if you were still using background image, the div would not need a width setting in CSS, but the background image would be sized using background properties.
- The whole card should have padding. The child elements within the card should only have vertical margins. Make sure you understand the difference between padding and margin.
- there is no need for a content div at all in this challenge, and it definitely would not need an explicit width.
- this follows on from the point above about including a link. Remember that anywhere you see a hover style in a design. It means that there is an interactive element there. Never just add cursor, pointer and hover styles to non-interactive elements as they won’t actually function.
- delete the media query and everything within it in the CSS. This challenge does not require a media query.
- when you do need media queries in future challenges, make sure you know how to use them well. You should be styling mobile first that means the mobile styles are the default CSS and the larger screen overrides, go within a min width media query. Media queries must always be defined in rem or em not pixels.
Marked as helpful