Article preview component using SASS & BEM

Community feedback

• 

  Samuel Palacios•615

  @samuelpalaciosdevPosted over 3 years ago

  Hi, Adam👋

  Great job on this challenge. Your solution looks good and it scales pretty well👍

  I only suggest some things 😉:

  • Scaling the card to make it look more like the original design, setting transform: scale(0.8); on the .card element.

  • Setting the furniture image as a background-image, it will help you, you can control it better than putting that img on the HTML Markup.

  I hope you have a nice day, keep coding!💙

  1

  ---

  

  Adam•430

  @aemann2Posted over 3 years ago

  @samuelpalaciosdev thanks for the advice. I still have trouble figuring out when to set an img in the markup vs. when to use background-img.

  Is there some kind of general rule on when you'd want to set an img in the html? It seems like it's usually better to just set images using CSS because you have more control, even though that seems counterintuitive to me...

  0

  

  Samuel Palacios•615

  @samuelpalaciosdevPosted over 3 years ago

  What a good question.

  I suggest you to read this thread on Stackoverflow

  Hope you have a nice day, bro💙

  1
• 

  Agata Liberska•3,995

  @AgataLiberskaPosted over 3 years ago

  Hi @aemann2, I don't think the photo needs to look exactly as it does in the design, but you could set it to as background-image of your div instead of putting it in your HTML, that gives you the option of adjusting background-size and background-position as needed. Just a suggestion :)

  Also - it would be great if the social media icons had some hover states - you can achieve that if you add the svg file using <svg> tag.

  And as per JS - you could separate your code into functions, like 'openModal' and 'closeModal' but I'm not sure that's necessary here, since it's only used once. Maybe someone more experience could advise, I'd be happy to learn, too :)

  1

  ---

  

  Adam•430

  @aemann2Posted over 3 years ago

  @AgataLiberska thanks for the suggestion, the other person to comment also suggested setting the img to background-image. I'm going to dig into the stack overflow thread he linked so I can understand in what situations to use which.

  0