Skip to content
Submitted 3 months ago

Responsive Testimonial Grid Section using CSS Grid and Flexbox

P
M Rafay•110
@the-Exalter
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Testimonials grid section
  • HTML
  • CSS
2junior

Design comparison

SolutionDesign

Community feedback

  • P
    Kamran Kiani•2,740
    @kaamiikPosted 3 months ago

    Hi. Some notes to improve your solution:

    1. You can check most of the pages and see they have a structure like this inside the body:
    <body>
  <header>...<header>
  <main>...<main>
  <footer>...<footer>
</body>

    Based on your design you may have header and footer or not. But You should have a main element inside your page. So after your body always wrap all of your code inside a main element. In this design you do not need header and footer.

    1. Firstly, profile images don't require alt text, so you can leave the alt attribute empty. Additionally, avoid using words like image, picture, or photo in the alt text description, as users with screen readers are already aware that it's an image.
    1. Personally I think your .highlight is a heading and a proper heading level is h2.
    1. For each card, you have a quote. Instead of using a <p> tag for this, you can use the <blockquote> tag, which is more semantic and appropriate for this context. I also have a suggestion for your <blockquote>. One use case of the pseudo-classes ::before and ::after is to add quotation marks before and after the text using the content property. This approach ensures that when selecting text with the mouse, the quotation marks are not included. Here is the code:
    blockquote::before {
   content: '“'; 
/* Additional styling */
  }

 blockquote::after { 
  content: '”'; 
/* Additional styling */ 
  }
    1. Try to use a proper CSS reset at the start of your CSS style. Andy Bell and Josh Comeau both have a good one. You can simply search on the internet to find them.
    1. Never limit your width and height in a container or element or tag that contains text inside. When you limit the width and height of elements containing text, you risk the text being cut off, overflowing, or becoming unreadable, especially on smaller screens or when the text dynamically changes. It's generally better to allow the container to adjust its size based on its content or set a flexible size that can adapt to different screen sizes and text lengths. You only need max-width here because it prevents elements from stretching beyond a certain point, keeping them visually appealing across different screen sizes. It ensures your design remains adaptive and doesn't get too wide on larger screens.
    1. For your grid, it's better to use repeat(4, 1fr) for your columns, or you can use grid-auto-columns: 1fr;. For the rows, I noticed that your code is more complicated than necessary. You only need two rows, so avoid splitting them with percentage units. I also suggest using grid-template-areas, which is more suitable for this context. You can search and find the good resources to learn them on the net.
    1. Your font-size and max-width should be in rem unit not px. You can read this article about it and why you should not use px as a font-size.
    1. you used many unnecessary codes in the media queries. Review your code and simplify it. It's not needed at all.
    0
  • jsemenborodas•110
    @jsemenborodasPosted 3 months ago

    Hello! You have some minor problems with the block sizes, but overall it looks pretty good.

    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord