Skip to content
OverviewSolutions
2
Comments
4
Kyle
@KylesTech95

All comments

  • Kyle•80
    @KylesTech95Posted about 2 years ago

    Hi Lewis!

    Great job on this project! I truly appreciate the use of CSS flexbox, Grid and Media Query for your design. Although your CSS look sharp, I thought I would share an addition to your preview card that would affect the presentation when moving in and out of the mobile/desktop View.

    • Image Fade-in/Fade-out when switching between device screens.

    Simply program the animation property's name, time, and the animation-fill-mode. Upon completion, program a @keyframes selector with the [animation name]. Programming the keyframe's opacity alone will make the image appear from a transparent state to its full state with the ease-in/ease-out function. ITS SO COOL!

    Desktop:

    img {
  max-width: 100%;
    /*added animation*/
  animation: appearIn .45s ease-in-out forwards; 
}
@keyframes appearIn {
  from {
    opacity: 0;
  }

  to {
    opacity: 100%;
  }
 }

    Mobile:

    picture img {
    height: 100%;
    /*added animation*/
    animation: appearOut .45s ease-in-out forwards; 
  }
  @keyframes appearOut {
    from {
      opacity: 0;
    }
  
    to {
      opacity: 100%;
    }
   }

  .container p {
    margin-bottom: 1rem;
  }
}

    I hope this addition helps! Great work!

    Marked as helpful
    1
  • Kyle•80
    @KylesTech95Posted about 2 years ago

    Hi Mariano!

    Great job on this project! I truly appreciate the use of CSS Grid for creating the preview card. Although your CSS look sharp, there is one addition I would like to add to your project in regard to device response.

    -Added below is a media query for the desktop view. The Css on both desktop.css & style.css works out to a wonderful & responsive solution. I would suggest combining the two files with a media query in so users can interchange between the two widths.

      @media (min-width: 1440px) {
    /*Imported desktop.css from github*/
    body {
      width: 100vw;
      height: 100vh;
    }
    
    .card-container {
      width: 38%;
      height: 400px;
      display: grid;
      grid-template-columns: 1fr 1fr;
      grid-template-rows: 1fr 1fr 1fr 1fr 1fr;
    }
    
    .card-container .card-container--header {
      grid-column: 1 / 2;
      grid-row: 1 / 6;
      background-image: url("images/image-product-desktop.jpg");
      height: 100%;
    }
    
    .card-container .card-container-title {
      grid-column: 2 / 3;
      grid-row: 1 / 2;
      font-size: 1.1rem;
      padding: 30px 25px 0 25px;
      margin: 0;
    }
    
    .card-container .card-container-product-name {
      grid-column: 2 / 3;
      grid-row: 2 / 3;
      font-size: 2.8rem;
      line-height: 3rem;
      padding: 0 45px 0 25px;
      margin: 0 0 20px 0;
    }
    
    .card-container .card-container-description {
      grid-column: 2 / 3;
      grid-row: 3 / 4;
      font-size: 1.3rem;
      line-height: 1.8rem;
      padding: 0 25px;
    }
    .card-container .card-container-prices {
      grid-column: 2 / 3;
      grid-row: 4 / 5;
      padding: 0 25px;
      margin: 20px 0 0;
    }
    
    .btn {
      grid-column: 2 / 3;
      grid-row: 5 / 6;
      margin-top: 0px;
      width: 80%;
      height: 45px;
    }
  }

    Great work, and I hope this helps!

    Marked as helpful
    0
  • KT•20
    @Yoh1Submitted about 2 years ago

    give me your opinion and tips

    flexbox

    2
    Kyle•80
    @KylesTech95Posted about 2 years ago

    Hi KT,

    Great job on this project! I truly appreciate the use of flexbox for creating the preview card. Although your CSS look sharp, there is always room for improvement.

    Issues:

    • I noticed that you included a media screen at the bottom of your styles sheet. I tweaked the width just to get the width of each element to line up as close as the design:
     @media screen and (min-width:1140px) {
.card{
    /*width: 375px;*/
    /*reducing the width will alow each element to line up just like how its places in the design*/
    width: 250px;
}
}
    • The [body] selector has been modified to adjust the image to the center, as well as included the font & text-align properties. Doing this will reduce the amount of lines used for each element.
    body{
    min-height: 100vh;
    background-color:hsl(212, 45%, 89%);
   /* width: 100vw;*/
    display: flex;
    /*addeed flex direction column*/
    flex-direction: column;
    justify-content: center;
    align-items: center;
    /*margin: 50px*/
    /*font-fproperties & text-align can be added to reduce the line space*/
    font-family: 'Outfit', sans-serif;
    font-size: 15px;
    text-align: center;
}
    • Selectors not selected
    /* text & p did not have the class "." in front of it.*/
.img, .text, .p {
    width: max-content; 
/*Not necessary because the content will adjust to the width of its parent [.card]*/
     text-align: center;}
/*in addition, you can eliminate these class selectors and paste the text-align property in the [body] selector*/
    • The paragraph text has been modified for text visibility as well as eliminating properties. In addition, the :root selector would be an effective tool to use to quickly change colors, fonts and sizes from the selector itself verses each individual element.
    .p {
    /*color: hsl(212, 45%, 89%);*/
    /*Paragraph Color that can easily be seen*/
    color: hsl(220, 15%, 55%);
   /* font-size: 15px;*/
    padding-top: 1em;
    /*font-family: 'Outfit', sans-serif;*/
}

    Root Example:

    :root {
  --bg-color: hsl(1, 1%, 100%);
  --bg-color2: hsl(212, 45%, 89%);
  --background-color3: hsl(218, 44%, 22%);
  --paragraph-color: hsl(220, 15%, 55%);
  --font-family:'Outfit', sans-serif;
  /*Weights 400 & 700*/
}

    I hope this helps!

    0
  • Kyle•80
    @KylesTech95Posted about 2 years ago

    Hi Raul,

    Great job on this project! Although your HTML & CSS look sharp, there is always room for improvement.

    • I understand that the goal was to solely style your project, however, the element used for the ["price"] class seems to be preventing the $169.99 from looking like the preferred font-size in the desktop design.

    • Alternatively, I wrapped both contents separately, one for [h2.discounted-price] & one for [p.original-price]. Both elements are styled by setting their display to inline, and then styling both selections individually. Please view the changes made below:

    <h2 class="discounted-price">$149.99</h2>
<p class="original-price">$169.99</p>

    /*added inline-block to place the h2 & p tags side-by-side*/
.discounted-price, .original-price {
/*color: var(--Dark-cyan);
font-size: var(--font-fraunces);
font-size: 30px;
font-weight: 700;
padding-bottom: 22px;*/
display: inline-block;
}
h2.discounted-price {
font-family: var(--font-fraunces);
font-size: 30px;
color: var(--Dark-cyan);
}
p.original-price {
color: var(--Dark-grayish-blue);
text-decoration: line-through;
padding-left: 2rem;
}
    • When it comes to flexbox, it is crucial to understand what is being flexed and how certain elements respond to flexbox. Rather than setting the flex direction to column, set the flex direction to row by deleting the column attribute. Reason being is that flex-row will span both [div] elements side-by-side. In other words, the "hero-image" will be right next to "card-info". Please view the changes made below:
    .container {
display: flex;
max-width: 100%;
max-height: 55%;
margin: 0 auto;
padding: 0;
border-radius: 10px;
overflow: hidden;
}

    I hope this helps!

    Marked as helpful
    0