Four card Responsive flex page

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 3 years ago

  👋Hi Daniel!

  l have some feedback on this solution:

  • Reliable, efficient delivery. Powered by Technology. should be in a single heading tag. Use span to style it differently.
  • The parent for all the cards should be a ul tag and wrap each list item with li so you don't need heading tag for the card title.
  • You can use strong tag for the card title.

  <p><strong>Supervisor</strong></p>
  

  • For any decorative images, you should leave the alt="" empty and add aria-hidden="true" or just role="presentation". It will make sure that the screen reader ignore those images. In this case, all images (the icons) are decorative only.
  • Use rem or sometimes em units instead of px. Using px will prevent the user to set the size of your page based on their needs.

  That's it! Hopefully this is helpful!

  Marked as helpful

  0

  ---

  

  Daniel Jansen van Vuuren•120

  @DanielJvVPosted about 3 years ago

  @vanzasetia Thank you very much for all the advice. Just one question. I didn't use much px only when I set my body's font-size and my box shadows. Should I have used rem there as well. How would that even work?

  1

  

  Aakash Verma•6,710

  @skyv26Posted about 3 years ago

  @DanielJvV basically 1rem is by default equals to 16px. So where ever you use 1rem or 2rem then it means you are using 16px and 32px there. Even you can change your default property of 1rem equal to 10px by setting the font-size: 62.5% in html property inside stylesheet like below

  html { font-size: 62.5%; }

  I hope you will understand it. Don't worry about what you have used, just focus on what you can do next in order to achieve better result with good approach.

  Marked as helpful

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 3 years ago

  @DanielJvV Yes, I would recommend you to use rem unit for body font size and box-shadow. It will make sure that everything will scale correctly based on the user setting. It's also important for accessibility 😉.

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 3 years ago

  @skyv26 I don't recommend to change the root font size to 62.5%, it comes with a lot of risk. Here's what an accessibility expert has said about it.

  Grace Snow said that on this solution:

  Yeah, I would never resize html/root font size down to 62.5% mainly because it is completely unnecessary, and it carries risk. You are correct you can mitigate against the accessibility issues by scaling the font size back up with 1.6rem on the body, BUT

  • not all elements inherit font size from the body so you have to remember to manually scale up them as well
  • you need to do extra testing for that reason
  • any rem sizes on third party components or plugins you bring in will need loads more overrides because you’re changing an established default, you cannot guarantee it will work correctly for everyone on every device, operating system, browser and tech set up Basically it comes with a lot of risks for almost no benefit.

  There is no reason 1rem needs to equal 10px. As soon as you start thinking entirely in REM - building for alignment, proportionality and spacing - rather than thinking in pixels, the whole point disappears.

  Marked as helpful

  0
• 

  T

  Grace•31,270

  @grace-snowPosted about 3 years ago

  Hi

  It’s a shame you’ve gone for flex on this when it is a perfect layout for css grid

  I recommend you tweak the html on this, whatever layout method you use

  • the main title is one h1 not a h1 then h2. They are read together to make sense
  • once that’s changed, you’d need to increase the other heading levels to h2
  • images don’t need wrapping in divs. Make them display block rather than wrapping in extra html that’s not needed
  • the images are decorative and do not need alt descriptions. Leave the alt values blank on those
  • you are misunderstanding the section element. This whole component could be a section, not the individual cards (they are too short in content) and certainly not the extra column you’ve added to be able to achieve the layout using flex (that wouldn’t be needed with css grid). When elements are only being added for styling/layout it is fine to use divs

  I hope this helps you

  Marked as helpful

  0
• 

  Aakash Verma•6,710

  @skyv26Posted about 3 years ago

  Well, your solution is not bad, if you didn't achieve the design perfectly that doesn't mean you haven't learnt anything, in desktop view your card width is smaller as compared to design preview and the reason behind is grid column width is less. You have set text-align center that's why your card text is in center. Well finally I want to say, experience comes with time and knowledge. So keep learning ;)

  Marked as helpful

  0
• 

  Daniel Jansen van Vuuren•120

  @DanielJvVPosted about 3 years ago

  @vanzasetia Thank you for all the feedback. I have made some changes and updated it. I am happy with what I have now. I'll move on to the next challenge keeping everything I've learned in mind.

  0