Three Column Preview Card

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted almost 2 years ago

  Hello there 👋. Good job on completing the challenge !

  I have some feedback for you if you want to improve your code.

  HTML 📄:

  • You must use a level-one heading (h1) even though this is not a full-page challenge. You can create an '<h1>' element within your 'main' element that will be hidden visually but visible and readable by screen readers. The class "sr-only" hides content visually and here are the styles to copy. e.g.: <h1 class="sr-only">3-column preview card component</h1>

  • Not all images should have alt text. Car icons are for decoration purposes only, so they can be hidden from screen-readers by adding aria-hidden="true" and leaving its alt attribute empty:

    <img src="./images/icon-sedans.svg" alt aria-hidden="true">
    <img src="./images/icon-suvs.svg" alt aria-hidden="true" >
    <img src="./images/icon-luxury.svg" alt aria-hidden="true" >
    

  CSS 🎨:

  • You should use the cursor: pointer property to indicate that the element like a button or a link is clickable.

  I hope you find it useful! 😄 Above all, the solution you submitted is great!

  Happy coding!

  Marked as helpful

  2

  ---

  

  Tom•70

  @tplatt92Posted almost 2 years ago

  @MelvinAguilar

  Thank you for the helpful information. I never knew we could hide the H1 element and I have implemented the changes you have suggested.

  Many thanks again and have a great day.

  1
• 

  Nelson•2,390

  @nelsonleonePosted almost 2 years ago

  HELLO.....congrats on completing this challenge....well done 🎊🎊

  I just a have a little thing to say, It's based on the small car icons. Visually, they don't seem to send much message across(just for design). It would be good to set ...

  aria-hidden="true" on them , so screen-readers(AT) won't stress on what they are, therefore making your solution accessible.

  Hope this comment was helpful and you understood. Have fun coding

  Marked as helpful

  1
• 

  Ambe Mbong-Nwi Nchang•550

  @Ambe-Mbong-NwiPosted almost 2 years ago

  Hello, awesome job here.

  Your "buttons" were created with the incorrect element. When the user clicks on the button they should be directed to a different part of your site. The anchor tag will achieve this.

  <a href="/">Learn More</a>
  

  Note It's a violation of the HTML5 specification to include either an anchor tag inside a button tag, or a button tag inside an anchor tag. Interactive elements are not permitted to be children of one another, as it introduces ambiguity with regard to user intent.

  Some browsers (like Chrome) will support this practice and, I believe, give the outer tag precedence when a user clicks on the nested elements, but not all browsers support it to any degree, and because it's not in the spec no browser is ever going to be required to. You'd be severely restricting the efficiency and effectiveness of your web application's cross-browser compatibility if you do this.

  Therefore, it's preferable to style your link into a button.

  Marked as helpful

  1

  ---

  

  Tom•70

  @tplatt92Posted almost 2 years ago

  @Ambe-Mbong-Nwi Thank you for the information, I have implemented the changes as you have so kindly suggested.

  In future, is it best to use links styled as buttons for purposes of navigation and then buttons for forms?

  Many thanks in advance.

  0

  

  Ambe Mbong-Nwi Nchang•550

  @Ambe-Mbong-NwiPosted almost 2 years ago

  @tplatt92 Yes

  • Buttons are used for actions that affect the website’s front-end or back-end like form submission.
  • links are used for navigation and actions that don’t affect the website at all.

  You are welcome

  1
• 

  バレンタイン 😈•64,170

  @VCaramesPosted almost 2 years ago

  Hey there! 👋 Here are some suggestions to help improve your code:

  • Your CSS Reset is being underutilized. 😢 To fully maximize 💯 it, you will want to add more to it. Here are some examples that you can freely use: Josh Comeau Reset Eric Meyer Reset

  • ⚠️ Remove the width from the body as it is 100% by default and you will want to change height to min-height.

  • For improved accessibility 📈 for your content, it is best practice ✅ to use rem for your font-size and other property values. While em is best for media-queries. Using these units gives users the ability to scale elements up and down, relative to a set value.

  If you have any questions or need further clarification, feel free to reach out to me.

  Happy Coding! 🎆🎊🪅

  Marked as helpful

  0

  ---

  

  Tom•70

  @tplatt92Posted almost 2 years ago

  @vcarames Thank you for your suggestions, I have implemented them as such.

  Should I use REM for all property values including margin, padding etc? Then any changes I make when using media queries change it to em?

  Many thanks for your support.

  1

  

  バレンタイン 😈•64,170

  @VCaramesPosted almost 2 years ago

  @tplatt92

  Glad I could help! 😁

  For the most part yes, the only time you would use em on padding and margin is if you want then to inherit the font-size from the parent element; so it will be a case by case scenario.

  For media queries, always em.

  0