3 columns preview card

Community feedback

• 

  Ahmed Hany•220

  @ahmedhanyhPosted almost 2 years ago

  Hey JJ! Amazing work!

  There are some things you can do to improve your solution:

  • You can remove the unnecessary border-style: none; declaration in your css reset as the initial value for the border-style property is already none.
  • The <main> element doesn't need to be a flex container in this case as it contains only a single child element, the display: flex; declaration can be removed. To center the <main> element in the page, give the body a min-height: 100vh and use either flexbox or grid to center it both vertically and horizontally, like this:

  /* Using flexbox */
  body  {
    min-height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
  }
  

  /* Using grid */
  body {
    min-height: 100vh;
    display: grid;
    place-content: center;   /* or place-items: center; */
  }
  

  and remove the margins around the <main> element.

  • Try to use a layout method like flexbox or grid to layout/space the content inside .main_content-wrapper elements instead of using margins.
  • Set the border-radius on the .main_content element with overflow: hidden; instead of specifiying each element corner's radius, like so:

  .main_content {
    border-radius: 0.5rem;
    overflow: hidden;
  }
  

  • To implement the active states for the 'Learn More' buttons, give them a border, change their background-color to transparent and their text color when hovered over using the :hover pseudo-class as in this code snippet:

  a {
    ...
    border: 2px solid var(--color-bg-hd-btn);
  }
  
  a:hover {
    background-color: transparent  /* or initial */
    color: var(--color-bg-hd-btn);
  }
  

  That's all I have. I hope you found the review helpful and I wish you the best with your future challenges.

  Marked as helpful

  0

  ---

  

  JJ•160

  @JeremyPaymalPosted almost 2 years ago

  @ahmedhanyh Hello !

  I did the modifications and it works perfect !

  Thanks for all the advices ! It was very helpful.

  JJ

  1

  

  Ahmed Hany•220

  @ahmedhanyhPosted almost 2 years ago

  @JeremyPaymal You're welcome! Glad it worked!

  0
• 

  Adriano•38,330

  @AdrianoEscarabotePosted almost 2 years ago

  Hello JJ, how are you? I truly loved your project's outcome, however I have some advice that I hope you'll find useful:

  To align some content in the center of the screen, always prefer to use display: flex; it will make the layout more responsive!

  Example:

  body {
      margin: 0;
      padding: 0;
      display: flex;
      align-items: center;
      flex-direction: column;
      justify-content: center;
      min-height: 100vh;
  }
  

  I noticed that you used 2 h1 tags, this is not recommended as we can only have one h1 tag per page to inform the main title, prefer to use h2 since this challenge is only one component!

  The remainder is excellent.

  I hope it's useful. 👍

  Marked as helpful

  0