3-column preview - CSS Grid and SVG sprite used

Community feedback

• 

  T

  Grace•31,090

  @grace-snowPosted about 3 years ago

  Hello

  This doesn’t fit on my mobile screen. You’d be better having some padding on body and setting a max width on the cards I think, letting them fill 100% up to that size

  I’m not sure why you’ve used transform translate to center the component on desktop when you already had grid on the cards. With grid it’s very easy to center content.

  The html on this looks really good. The only things I think you need to change are

  • h2s instead of h1s. You would only have one h1 on a page usually. If you want to treat this as if it was a real web page you could add an sr-only h1 above the cards component if you wanted to, but it doesn’t matter so much on this.
  • the svgs need aria-hidden true on them
  • the SUVs heading needs to be written in the html as it would be pronounced. SUVs would be read by screenreaders as S-U-Vs. But if you kept it as Suvs some screenreaders may read it as if it’s a word.

  Good luck

  Marked as helpful

  0

  ---

  

  P

  Arjen Combee•50

  @ArCombeePosted about 3 years ago

  @grace-snow Wow! Thanks for your support and feedback, I really enjoy reading your comments. The last couple of day's, I learned so many new things based on this feedback 🙏.

  • About the mobile thing, I did test on my devices and did not see this. Today I found a website to test on other devices, and see that devices behave different. So -> Refactoring today 😮

  • First I had H2, switched it to H1 but don't know why if I'm honest. Need to see the challenges as part of a whole page and not as the page.

  • The SVG tip I will explore, ARIA is new to me, did read some topics and will read more today, thanks for pointing this out.

  • Screen readers! Did not think of them, at all. This needs more exploring on my side. I'm somewhat ashamed that I didn't think of this, like color-blindness.

  Today I will refactor this project and upload the new version based on this great feedback.

  Thanks again, really appreciate the time you spent on giving me the chance to do better the next time.

  -- Arjen

  1

  

  T

  Grace•31,090

  @grace-snowPosted about 3 years ago

  @ArCombee while you are refactoring anyway, I should mention the border radiuses don’t look quite right on mobile. I’m seeing a curve on top corners on all coloured boxes which is leaving a little space. I don’t think it was in the design like that.

  You may be able to set the border radius on the cards wrapper and use overflow hidden to create the radius that would be right for all screen sizes instead of setting on each card.

  Good luck

  Marked as helpful

  0
• 

  P

  Arjen Combee•50

  @ArCombeePosted about 3 years ago

  First revision is a fact 👍. After the feedback I looked at the design files, the Figma design is different and has rounded borders on all the cards, they overlap slightly, so no bleed of underling card is visible.

  But when I look at the JPG files, or open the files in Adobe XD you don't see that. So refactoring was done, and... well let me know if it looks like the original (somewhat).

  I'm happy with the result, and learned new things altogether, and that is why I'm here on this service.

  Best Regards,

  Arjen

  0