Design comparison
Solution retrospective
Hey everyone!
I've tried to use CSS Grid for this challenge. Mobile view was relatively easy, but I felt like I ran into more issues when updating the Grid rules for the desktop styling. I've used grid gap in certain places, where padding / margin may have been better.
In particular, getting the cards at the bottom to look good in different widths & heights was one of the things I had issues with. I'm looking for any general suggestions to improve how I have used Grid. I think next time, I may use Grid for the general layout, and Flexbox for the specific sections.
Thanks!
Community feedback
- @ApplePieGiraffePosted over 3 years ago
Greetings, once again Shivam! 👋
Well done on yet another challenge! 👍 Like grace-snow said, your solution's great! 😀
A super tiny suggestion from me would be to not forget to add some space between the testimonial cards and the bottom of the page in the desktop layout so there is always some room between the two (even on devices with screens that have smaller heights). You could easily do so with some margin/padding. 😉
Keep coding (and happy coding, too)! 😁
1 - @shivjoshi1996Posted over 3 years ago
Hey!
Ah yes - I forgot about the h3 I added without a h2, thank you for catching that! Also good point on the divs, I think I added them just to "visualise" the structure a bit more clearly, next time I'll make sure to take note of any unnecessary divs.
Thank you for the feedback on Grid - I added a "display:grid" to the body in order to center the main grid container, I tried a few other things but they didn't seem to work at wider breakpoints, and the grid would remain on the left-hand side. I'm sure there's a way to center the grid itself in a much easier way, I just couldn't figure it out.
In terms of making each review a direct child of the main grid, are you suggesting that I should have had more columns & rows on the main grid? (i.e. a 12-column grid so that I can accurately place each of the staggered star reviews on the grid, for example). Just want to make sure I'm interpreting that correctly.
I'll be sure to give grid template areas a shot next time!
Thanks again for all of the feedback, really appreciate it.
1 - @grace-snowPosted over 3 years ago
Hi
I think this solution looks really nice. The only thing I'm not sure about in the html is a few places where paragraphs have been wrapped in divs - I would probably put the classes on the paragraphs themselves. But not really an issue.
Other thing in html is you're missing a heading level. I would add a visually hidden h2 for the reviews section to fix the page structure for assistive technologies and search engines.
With the css grid stuff, I think you're making it a little hard for yourself at the moment. I'm not sure why the body is grid, and if you're going to use grid for the rest of this layout I would make each review a direct child of the main layout grid. For example, you could leverage css grid to create the staggered effects on the reviews. I'm not sure this is the best challenge for grid otherwise, as there is little benefit from. Only using it to lay out some wrapper containers.
You may find it easier next time you use grid to try grid template areas rather than line numbers. It's much easier to visualise that way I think.
Anyway these are all just thoughts and ideas really because this works great as it is
1
Please log in to post a comment
Log in with GitHubJoin 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