3 Columns layout with responsive features using CSS Grid and Flexbox

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello caastech, Congratulation on completing another project .
  Your solution is really nice. I have few suggestions : HTML:

  • Document should have one main landmark. Wrap the body content in< main>tag read more about main landmark

  • You can replace the <div class="attribution">by a <footer > tag and it would be out the < main>.

  • Add hover effect on the Links.

  • Whenever you include interactive elements (button, links, input, textarea), make sure you include clearly visible focus-visible styles as well as hover ones.This will make the users can navigate this website using keyboard (by using Tab key) easily .

  • For any decorative images, each img tag should have empty ``alt="" andaria-hidden="true" ` attributes to make all web assistive technologies such as screen reader ignore those images. (in this case all the images are decorative).

  • To get rid of the accessibility issues you can add a <h1> with class="sr-only"(Hidden visually, but present for assistive tech).

  .sr-only {
  	border: 0 !important;
  	clip: rect(1px, 1px, 1px, 1px) !important; /* 1 */
  	-webkit-clip-path: inset(50%) !important;
  		clip-path: inset(50%) !important;  /* 2 */
  	height: 1px !important;
  	margin: -1px !important;
  	overflow: hidden !important;
  	padding: 0 !important;
  	position: absolute !important;
  	width: 1px !important;
  	white-space: nowrap !important;            /* 3 */
  }
  

  To read more This fairly modern technique will hide or clip content that does not fit into a 1-pixel visible area. Like off-screen content, it will be visually hidden but still readable by modern screen readers.

  • You can replace the <h1 > by <h2>.

  • Your solution is not responsive.

  I really this feedback helps , happy and keep coding.

  Marked as helpful

  1

  ---

  

  cacosted•570

  @cacostedPosted almost 3 years ago

  @PhoenixDev22 Thanks, new things to consider I'll look forward to implemented

  0
• 

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  👋 Hi there!

  🎉 Congratulations on reworking this challenge! Hopefully, you learned something new when re-doing this challenge. PhoenixDev22 has given some incredible feedback and here are from me:

  • Accessibility
    • These car icons are decorative. To know that the images are decorative or informative (add meaning), you can try to make all the images hidden and now you see the content of your page. If you don't miss any information (still understand the page content), you can assume that all the images are decorative images.
    • Some resources to learn about the alternative text and images.
      • Axess Lab | Alt-texts: The Ultimate Guide
      • Images Concepts • WAI Web Accessibility Tutorials
    • Use CSS to uppercase the text. The uppercased word in the HTML will be spelled by the screen reader (spelled letter by letter).
  • Styling
    • (Personal Preference) To make the card perfectly in the middle of the page, you can simply make the body element as a flexbox container. You don't need to make the container as a grid item.

  /**
   * 1. Make the card vertically center and
   *    allow the body element to grow if needed
   */
  body {
    display: flex;
    align-items: center;
    justify-content: center;
    min-height: 100vh; /* 1 */
  }
  

  • I would recommend giving some padding on the body element to prevent the card from having full height on mobile view.

  That's it! Hopefully, this is helpful!

  Marked as helpful

  0

  ---

  

  cacosted•570

  @cacostedPosted almost 3 years ago

  @vanzasetia Thank you very much, I'll give it a look at these suggestions.

  0