Blog Preview Card

Community feedback

• 

  geomydas•1,400

  @geomydasPosted 2 months ago

  I just finished reading your code, it seems good but has some critical issues.

  • Add a box shadow to the card
  • Add a border to the card
  • Don't use inline styles. Line 28, in your HTML you can just add classes to the avatar image and style it there. Inline styles are bad practice since they have very high specificity, hard to override, and is not reusable.
  • Use a proper CSS reset. It makes your CSS look the same in most browsers. I reccomend using Josh Comeau's or Andy Bell's CSS reset as most people use them. You just copy and paste them in your CSS and you are all good
  • No need to set a base font size in the body, its the default already plus setting font sizes in rem is an accesiblity issue
  • Don't use ids for styling elements in CSS, use classes instead. Using ids are bad practice as they are not reusable and have very high specificity, you should always strive for the lowest possible specificity in your CSS. You can read in this article the appropriate use cases of ids
  • Use proper semantic HTML
  • Remove the article tag in your HTML and instead replace it with a main tag. A website should always have atleast 1 main landmark for accesibility. Article tag for this doesn't make any sense as you would typically use them for blogs, articles and any text heavy part of a website.
  • Don't skip heading levels. You should replace the h2 with an h1 tag, heading levels should always decrement or increment by 1. For example, using an h3 tag without having an h2 tag around it is semantically inappropriate HTML
  • Remove the width: 90% in your card container, replace it with an optional width: 100%. Whenever I see widths that are not full, I am often confused if the width is too much or too small. Also, you already have a max-width already. You would typicall use widths that are not full in sections with multiple items that need to have the equal width but you can use CSS grid for that
  • Replace #learning-tag in your HTML with a link because in a real site it would take in you to a section with a learning topic. Also, <b>, <span> elements are not semantic HTML. They are only used for styling purposes. You can use the <strong> tag in place of <b> instead.
  • Replace #card-body-date with a paragraph instead. Div's dont carry meaning like spans aswell so not semantic HTML
  • Don't overuse divs unless you really need to. You can simply apply the styles to the child itself instead of nesting it in a div. #card-header, #card-body, #card-body-header, #card-body-date has no need since you could've just apply the styles directly
  • Use the rem unit in place of px most of the time. You would typicall use the px unit for small stuff such as borders, outlines and shadows. You would use rem for all of the other stuff. But if you want to go deeper, you would use rem if you want it to scale with user's set font size which is the case most of the time anyways/
  • No need for justify-content: flex-start as it is the default in flex containers. Omit them
  • Remove all of the media queries as this project doesn't need it
  • Use numbers instead of words in setting font-weight to be consistent. Instead of bold, use 700
  • Remove the second font-face declaration at the top of your CSS file as no text here is italic and it bloats your file size by fetching resources that wont be used anyways

  With that said, refactor your code by each part and your code will be much better. It may feel overwhelming at first but in the future projects, it will be smooth sailing

  Have a great day and have fun coding!

  Marked as helpful

  1