Blog preview card - HTML, SCSS

Community feedback

• 

  Kristian Kalvå•360

  @kkalvaaPosted 6 months ago

  Good updates, here are acouple thoughts on the new stuff:

  1: Since this is the only thing on the page it is technically correct to use H1. However, this component would be a part of a page and as such the card title should probably be H2 or H3, depending on the page hierarchy. But again, it is technically correct on this single page to use H1.

  2: A big point of BEM is to keep specificity as low as possible. Instead of ".card .card__avatar" (0 2 0) it should just be ".card__avatar" (0 1 0). The lower you keep specificity the easier it is to override it later. If you were to have a ".card__avatar--square" class, or --big/--small/etc., you would now have to also up the specificity to 0 2 0 in order to override ".card .card__avatar". This quickly leads to a specificity war and you'll find yourself overqualifying your selectors more than you need to. It doesn't matter too much in a css-file of this size, but it does matter when you scale up, especially when you add more people to a project. In my experience almost all problems in css is a direct result of specificity problems.

  Marked as helpful

  0

  ---

  

  P

  KN•170

  @kaiens-labPosted 6 months ago

  @kkalvaa Thank you for providing such comprehensive advice! I have already made the modifications and referred to the version you provided on GitHub (I really appreciate everything you have done).

  0
• 

  Kristian Kalvå•360

  @kkalvaaPosted 6 months ago

  A couple thoughts:

  1: You could consider adding a transition to the hover effect on the card itself. Having the shadow instantly move to its hoverstate is a bit jarring. Try looking into transition

  2: You should look into BEM style naming for your classes. Instead of "card" and "card-text"/"card-avatar" it should be "card__text" and "card__avatar". You're halfway there already, might as well actually adopt the actual naming system. A good primer

  3: You shouldn't use H5 without previously having used H1, H2, etc. on the page. H5 means heading level 5, which means it should be a subheading of four previous levels of headings. Unless you're marking up a scientific paper there is a very high chance you will never need to go below H3 on a normal web page.

  3B: Learning shouldn't be a heading at all, it is a tag, not a heading. Same with the date, not a heading, but should go in <time></time>. "HTML & CSS foundations" is the actual title/heading in this design.

  4: The Sass looks good, I would look into replacing Sass variables with css custom properties these days as they are more flexible.

  5: Magic numbers and why you should avoid them

  Marked as helpful

  0

  ---

  

  P

  KN•170

  @kaiens-labPosted 6 months ago

  @kkalvaa Thank you so much for your advice and the reference link! I've followed your suggestions and made the changes accordingly.

  0