Mobile first using CSS Grids with flexbox

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout in desktop looks great, though the responsive state could be better since when changing the screen's width, the button gets distorted. The mobile layout however looks really great.

  You already got great feedback from others and is really great. Just going to add some quick suggestions:

  • You don't need to use section to wrap each card since they all are related to one another, if you are going to use section use it to wrap the whole 3 cards since it is a "section" of card-colllections.
  • Always have a h1 on a page. Since there is no visible text that could be a heading tag, you will need to make the h1 screen-reader only. Meaning it will be using sr-only class. Have a look at Grace's solution on this one inspect the layout and see how she used the h1, copy also the styling on it, you will use that a lot.
  • Each img should be hidden since they are just decorative images. So you need to use alt="" and extra aria-hidden="true" attribute on it. Decorative image should be hidden at all times.
  • Also, there are lots of div wrapping every content, you don't really need those lots, you could remove them^^

  Aside from those, really great work on this one again.

  Marked as helpful

  1
• 

  Samia Mahwash•170

  @Mahwash-AlyPosted about 3 years ago

  Nice Try! I myself just attempted this challenge. About the second point that CyrusKabir mentioned, you can use in body is: display: flex; align-items: center; justify-content: center; height: 100vh;

  This should solve the issue of centering container.

  Marked as helpful

  1

  ---

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @SamiaMahwash Hey, just to ughm make sure, using height: 100vh is not that great since it limits the element's height based on the remaining screen's height. Instead it would be better to suggest using min-height: 100vh as this takes full height but let the element expand if needed.

  Just a small correction but still awesome that lots of people are giving suggestions to one another^^

  Marked as helpful

  1

  

  Samia Mahwash•170

  @Mahwash-AlyPosted about 3 years ago

  @pikamart thanks. I didn't know about this.

  Marked as helpful

  1

  

  Samia Mahwash•170

  @Mahwash-AlyPosted about 3 years ago

  @pikamart I would be grateful if you could look at my solution of the same challenge. I am having issues with media query and it's driving me crazy now. TIA My solution: https://www.frontendmentor.io/solutions/3columnpreviewcard-TXHRpwiPd

  Marked as helpful

  1

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @SamiaMahwash :>> sure but maybe at around lunch time on my region. Going to design some in this morning but I will give yours as my first review for the day

  Marked as helpful

  0
• 

  CyrusKabir•1,885

  @CyrusKabirPosted about 3 years ago

  hello my dear friend ♥ actually you cards are good and clean but here some misings :

  • your buttons should have hover states
  • you need to research about centering ways in css and then try too fix the place of container in center of the page

  1