Skip to content
Submitted about 2 years ago

Responsive Grid layout

Shiva•670
@shivaprakash-sudo
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

Solution retrospective

Hello everyone,

I have done several projects before, but this is my first project on this website, feel free to check it out and please let me know if there are any bugs or suggestions.

Regarding bugs, I have encountered one myself, which is related to media query. All media queries are working fine, except the one with min-width:780px, when I checked the dev tools in Firefox, it said that there's an error regarding grid-template-areas and that it is deprecated. I'm confused because the other queries are working fine, except that one. If anyone knows how to solve this, please kindly leave a comment.

Thank you.

Community feedback

  • Amos•470
    @fisttyPosted about 2 years ago

    Hi Shiva about "min-width:780px" the problem is that in the line below

    .testimonial-grid {
        grid-template-areas: 
        "one one two"
        "three five"
        "four five";
    }

    1. "three five" must have a third area name so "three five" should be "three five five" or "three five (anything)"

    2. Same thing applies to "four five", it should also be something like "four five five"

    3. If you want it to be empty then you can "." on the area. Example "three five ."

    Lastly <!DOCTYPE html> should be on-top of your html

    Marked as helpful
    1
    Shiva•670
    @shivaprakash-sudoPosted about 2 years ago

    Hello Amos,

    Thank you for the clear explanation, now I have a little bit more understanding of how CSS grid works.

    I checked the code, !DOCTYPE html is already at the top of the html.🤔

    1
  • Lucas 👾•104,400
    @correlucasPosted about 2 years ago

    👾Hello Shiva, congratulations for your solution!

    I can tell that you did a great job here, by the level of detail and semantics you've added here. For example, even the profile photo border purple you've added to the correct elements (the black and purple card) thats amazing, also that you've used the correct tags to wrap all the elements using <main> and <article>.

    If you want to improve it just a little bit more, you can wrap all the quote text with a <blockquote> instead of the <p> tag. This way your solution will be super semantic.

    Anyway Shiva, great work, hope it helps, happy coding!

    Marked as helpful
    0
    Shiva•670
    @shivaprakash-sudoPosted about 2 years ago

    Hello Lucas,

    I've surrounded the quote with <blockquote>, thank you for your kind words.

    1
  • David•8,000
    @DavidMorgadePosted about 2 years ago

    Hello man, congratulations finishing the challenge!

    The UI looks responsive and flows pretty well, the only thing I would change is the centering of all the content throw the body, instead of having it on the top side of the page, you can fix this pretty easy.

    Give your body a display: flex, flex-direction: column and justify-content: center, with this you will have all your content in the center of the page!.

    Hope my feedback helps you!

    Marked as helpful
    0
    Shiva•670
    @shivaprakash-sudoPosted about 2 years ago

    Hello David, thank you.

    Thank you for the suggestion, I changed the code, now it is centered both vertically and horizontally.

    😊

    0
  • Vanza Setia•27,775
    @vanzasetiaPosted about 2 years ago

    Hi, Shiva Prakash Pendem! 👋

    Could you give more information regarding the bug? Maybe, try to reproduce the bug so that I can see the issue on my browser.

    I check the MDN documentation for grid-template-areas and it is not a deprecated property. Also, as far as I know there are deprecated HTML tags such as marquee, etc but I don't know that CSS properties can also get deprecated.

    I took look at the site on my Chromium browser (v104) and Firefox browser (v102), and I didn't see any error in both consoles (usually error messages would appear in the console).

    Anyway, I have one or two suggestions. I would recommend making the h3as p element. It is not a sub-title or sub-heading. It is more likely like two paragraphs than a heading and a paragraph. Also, alternative text should not contain the word "image" as it is already an img element. Screen readers would pronounce it as an image so adding the word "image" to the alternative text can create more noise.

    I hope this helps. 😊

    Marked as helpful
    0
    Shiva•670
    @shivaprakash-sudoPosted about 2 years ago

    Hello @vanzasetia

    The bug is that the code I wrote under min-width:780px media query doesn't seem to effect the layout, so I commented it out, which is why you wouldn't have been able to seen the error in the console.

    I'll take your suggestions into consideration, thank you.

    It helped.😁

    1

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