3-column preview card component solution with fluid resizing

Community feedback

• 

  Roksolana•910

  @RoksolanaVeresPosted 10 months ago

  Hey, I've looked at your code and attempted to organize your css better to avoid duplication of the properties.

  First of all, you don't have to specify in .card those values that are unique for each card and which you still need to define for every card separately. Thus in this piece of code:

      .card {
        padding: 3em;
        display: flex;
        flex-direction: column;
        align-items: start;
        row-gap: 1rem;
        background-color: black;
      }
  

  you'd better delete the background-color property, since it will be unique for each of the cards. And here:

      .card::before {
        content: "";
        background-color: hsl(0, 0%, 31%);
        width: 2.5rem;
        height: 2.5rem;
        border-radius: 100vw;
      }
  

  we can get rid of content and background-color and just specify these properties for each card separately, like:

    .card--sedan {
          background-color: var(--color-bright-orange);
        }
  
        .card--suv {
          background-color: var(--color-dark-cyan);
        }
  
        .card--luxury {
          background-color: var(--color-very-dark-cyan);
        }
  

   .card--sedan::before {
          content: url(./images/icon-sedans.svg);
        }
  
        .card--suv::before {
          content: url(./images/icon-suvs.svg);
        }
  
        .card--luxury::before {
          content: url(./images/icon-luxury.svg);
        }
  

  By the way, this is all code you need for each card, cause all the other properties you already have in your general .card class. I mean, previously you had this:

      .card--luxury::before {
        content: url(/images/icon-luxury.svg);
        background-color: unset;
        width: auto;
        height: auto;
      }
  

  But since your general class .card::before already has a specified width and height you don't have to define them again for each of the cards.

  One more thing that I've noticed: your paths to the images don't work as they are intended to. The images didn't open on my computer. To fix it you need to add a dot before the first slash: content: url(./images/icon-sedans.svg);

  That's all, have a nice day and happy coding :)

  1

  ---

  

  Thomas Burette•190

  @tburettePosted 10 months ago

  @RoksolanaVeres, thanks for reviewing my code.

  I did put the code in .card and .card::before that looks like duplication there for a reason :). Let me explain!

  When writing a BEM component I want it to work even if there are no modifiers. I want not only <section class="card card--sedan"> to work. I also want a plain <section class="card"> to work. That's why there is that extra code.

  Of course in this challenge there are only ever going to be three section so there will always be a .card--sedan, .card--suv or .card--luxury. There will never be a "naked" .card in this challenge but that's a good practice I wanted to adhere to.

  2

  

  Roksolana•910

  @RoksolanaVeresPosted 10 months ago

  @tburette Oh, sorry then for my silly comment, I wasn't aware of these best practices. And thank you for the explanation, it absolutely makes sense now.

  0

  

  Thomas Burette•190

  @tburettePosted 10 months ago

  @RoksolanaVeres, If you are interested https://getbem.com/faq/ is an excellent resource regarding the details of BEM.

  2

  

  Roksolana•910

  @RoksolanaVeresPosted 10 months ago

  @tburette thanks, I'll definitely read it :)

  0