Blog Preview Card w/HTML and CSS

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 7 months ago

  Hello, good job!

  My suggestions:

  HTML:

  • You could place the svg as an <img> - an inline svg is only required when changing the colors. If the SVG has meaning, it needs an accessible name - add role="img" and add a title element inside with a descriptive name. If the SVG is decorative, add aria-hidden="true", so screen readers can ignore it.

  • Never have text in divs alone. "Learning" should be a <p>, it doesn't need to be wrapped in a <div> either.

  • I would use the <time> element for the date:<p>Published <time datetime="2023-12-21">21 Dec 2023</time></p>.

  • You don't need to wrap the elements in .parent-div in divs.

  • I would replace id with class.

  • Remember that screen readers recognize images by default, so you shouldn't include words like "image" or "photo" in the alt text. A suitable alt text for the profile image would be "Headshot of Gary Hooper".

  CSS:

  • Paragraphs have a default font-size of 1rem, so there is no need to declare that explicitly.

  • On "Learning", remove the max-width. To restrain its width, add display: inline-block. You can also remove the Flex properties - The top and bottom padding is equal, as is the left and right padding- hence, the text will get centered in the middle. I would add a bit more horizontal padding, though. If you change padding: 6px to padding: 6px 10px, this will add 6px on the top and bottom, and 10px on the left and right.

  Marked as helpful

  1

  ---

  

  P

  Mukarram Haq•270

  @MukarramHaqPosted 7 months ago

  @Islandstone89 Thank you for yet another detailed review of my code.

  I have a few questions, I tried googling them but, I couldn't find a straight answer:

  1. Is there a reason to not wrap the elements inside a .parent-div? Or is it because of this particular scenario?
  2. We can limit the width with max-width? I know that display:inline-block implies no specific width so why should we use that?

  1

  

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 7 months ago

  @MukarramHaq

  1. Divs are primarily used for grouping content. The profile image and text are both grouped in a div - there is no need to complicate the structure beyond that, especially since there are only 2 elements. If you had, let's say, 6 elements, and you wanted to separate element 1-3 from element 4-6, it would make sense to wrap each of those groups of elements in their own div.

  2. We should let the content determine the width of the "Learning" element. You can use max-width, but it is more used when you want to prevent lines of text from being too wide - using the ch unit, which defines a max amount of characters before the text should wrap. In this case, we don't need to limit the text, as it is very short. As mentioned the padding along with the content, will give the element the width it needs.

  Hope this makes sense :)

  0
• 

  sreos•110

  @sreosPosted 7 months ago

  Better than actual one

  1

  ---

  

  P

  Mukarram Haq•270

  @MukarramHaqPosted 7 months ago

  @sreos I appreciate that brother!

  0